Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Admin panel refactoring: Electric Boogaloo #43

Closed
wants to merge 51 commits into from

Conversation

ddavness
Copy link
Owner

@ddavness ddavness commented Feb 6, 2022

This is a deeper refactor on the admin panel. The main goals of this PR are:

  • Get rid of dead code;
  • Modernize all the code so that it conforms to some styling guide;
  • Make code more readable and uniform (i.e. for same thing, use the same libraries);
  • Improve the security and privacy of the panel;

There won't be many changes (if any) to the aesthetics of the panel on this PR.

@ddavness
Copy link
Owner Author

ddavness commented Feb 6, 2022

Code Guide in a nutshell for the management daemon

  • Identation done via tabs (hard tabs)
  • For the backend (Python):
    • Follow PEP8 (except the identation part)
    • Revert to using a cookie as an authentication ticket (so that we don't need to handle authentication on our own code);
    • Review the API interface - try to make it as consistent as possible;
  • For the frontend (HTML):
    • Keep everything reasonably idented;
    • let over var. Always.
    • No inline HTML in jQuery (e.g. no something.html("<p>Some html</p>")). Use templates instead;
    • .text() over .html() and you're not risking a XSS vulnerability;
    • Try using and abusing jQuery as much as possible (instead of the much more verbose document.getElementById() blah blah blah;
    • Id's must be unique. Use classes when you're creating templates;
    • Don't perform cross requests to other sites (client-side);

@ddavness
Copy link
Owner Author

The current authentication model of the admin panel can be done in three ways:

  • Session token sent via the Authentication header (this token is visible to JavaScript);
  • Username + PW combo (used for external API requests like dynamic DNS);
  • Specifically for Munin, authentication is done by issuing a short-lived cookie;

I'll be looking to overhaul this to make stuff more consistent (and likely more secure too):

  • Username + PW (+ 2FA when applicable) will now be only used for tokens to be issued (i.e. login only);
  • All tokens will take the shape of cookies (with the respective CSFR mitigations, of course);
  • In the future: Folks will be able to generate API keys specifically for external use;

@ddavness
Copy link
Owner Author

The new authentication model and flow seem to be working nicely so far :)

@ddavness
Copy link
Owner Author

Right now I'd be surprised if things work at this state 🙃

@ddavness
Copy link
Owner Author

ddavness commented Mar 3, 2022

There won't be many changes (if any) to the aesthetics of the panel on this PR.

Ok, I might have lied a bit when I said that. There will be some changes to the layout to make the user experience more consistent. But nothing too much out of the ordinary.

@ddavness
Copy link
Owner Author

ddavness commented Mar 8, 2022

One of the things also being worked on is a much more extensive form validation process. Forms will be validated client-side before any data is sent to the server (the data will of course be checked again server-side).

If any issues pop-up during validation, they'll be presented to the user like this:

image

The server should also provide it's own information in case server-side checks do fail.

@ddavness ddavness self-assigned this Mar 8, 2022
@ddavness
Copy link
Owner Author

That's aliases done. On to (checks notes) DNS!

@ddavness ddavness closed this Nov 6, 2022
@ddavness ddavness deleted the dev-admin-panel-refactor-deep branch November 6, 2022 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant