Add account info page and change up colors#902
Conversation
for more information, see https://pre-commit.ci
|
Okie it does work |
jaysa68
left a comment
jaysa68
left a comment
There was a problem hiding this comment.
account page looks great, and i am down 4 the color and font change.... fresh coat of paint
|
id like approval from a current SM though, since its a fairly forward-facing change |
Co-authored-by: sophie <sophiebsw@gmail.com>
Co-authored-by: sophie <sophiebsw@gmail.com>
|
Will go ahead and make those changes There is also a corresponding PR at ocf/mkdocs#66 Here's an example of the color/font difference:
I basically just wanted it to match our promotional materials, keyboards, etc. better by using blue as the accent color |
|
I think this seems ready to merge but I think @24apricots had some comments |
|
where |
| ssh = SSHClient() | ||
| host_keys = ssh.get_host_keys() | ||
| entry_ed25519 = HostKeyEntry.from_line( | ||
| 'ssh.ocf.berkeley.edu ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPm+RlDujsxQyxFTEOCTeImSBDvr63cL8Kg+rNrH6NK8', # noqa |
There was a problem hiding this comment.
why is this in two different files
|
|
||
| if not error: | ||
| if command_to_run == 'paper': | ||
| command_to_run = f"/run/current-system/sw/bin/paper view {username} | sed 's/\x1B\\[[0-9;]*[a-zA-Z]//g'" # noqa |
There was a problem hiding this comment.
arbitrary code execution. its minor because you authenticated as the username, but nonetheless
There was a problem hiding this comment.
also i think sed, which im assuming is here to remove ansi color stuff, is unnecessary if you set get_pty to False
There was a problem hiding this comment.
good point, this page is deprecated though so might as well just remove it?
There was a problem hiding this comment.
are these intentionally left unchanged?
There was a problem hiding this comment.
ngl i was just testing the paper command and idk if the page as a whole is worth updating
| COMMAND_CHOICES = ( | ||
| ( | ||
| '/opt/share/utils/bin/paper', | ||
| 'paper', |
There was a problem hiding this comment.
why was the path to the paper binary removed here and hard coded (with the correct path for nixos) in line 56?
There was a problem hiding this comment.
it's kind of a jank solution to add in the username
| @@ -45,6 +52,8 @@ def commands(request: HttpRequest) -> HttpResponse: | |||
| error = 'Authentication failed. Did you type the wrong username or password?' | |||
There was a problem hiding this comment.
(this line was not changed by you) this error may be confusing when the error is not related to an incorrect username or password
There was a problem hiding this comment.
(this line was not changed by you) why do we have them enter the username when they are already logged in and the username can be pulled from there? is this to support running commands as another user given their password?
There was a problem hiding this comment.
im not sure if we should allow that since they can just log in as that other user
|
considering the vast majority of the issues are from the basically deprecated commands page, do we want to just remove it tbh |
|
like it's probably a minor security risk just to keep around unmaintained and there's not rly any practical purpose for it |
|
sure |
|
running commands can already be done in a browser or any ssh client at ssh.ocf.berkeley.edu so that part is not needed. maybe we can link to it from the account status page? |
|
Ok bye bye commands |
|
merge the mkdocs pr too pls ^_^ |
5 participants
It will work once quota gets fixed trust me