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

Implement user accounts and token based authentication with CSRF protection #202

Merged
17 commits merged into from
Oct 16, 2020

Conversation

bernhard-herzog
Copy link
Contributor

Implement user accounts and token based authentication

This pull request adds a store for user accounts and session data and
implements sessions on top of that as outlined in the discussion for
#111.

The UI part is a bit unpolished in the current version, but it's working
and can at least serve as a starting point for discussing how to merge
this.

bernhardreiter and others added 8 commits June 11, 2020 13:02
 * Add the three original files from https://code.jquery.com/
   so the source code stays available, license is within the files
   itself. (This is followig the example of other third party source
   files in this repository.)
 * 3.2.0 -> 3.4.1, changed navbar style so that both images in the brand
   align again.
The session store uses SQLite to store the data and provides methods to
add user accounts, verify passwords and to start and verify session
tokens.
The server now requires authentication: The client has login with the
new login API function and then use the returned session token in
subsequent requests.

This commit implements the API part of this, using new reworked session
store for the user accounts and token implementation. The client is
modified to have a login button that uses a modal dialog to ask for
username and password, fetches a token from the server and stores it in
the browser's sessionStorage. The JS-initiated HTTP requests are now
done through the new authenticatedGetJson function in static.js which
retrieves the token and sends it in the Authorization header.

The implementation is still a little rough, particularly the UI part. In
particular, authentication is effectively optional in the sense that if
no session store has been configured, no login is required. However the
login button is always shown. Also the styling of login button and
dialog could be improved and some of the behavior could be better (e.g.
log out should return to the main page).
The command can be invoked like this:

   hug -m intelmq_manager.api -c add_user <username>

The command asks for the password for the account.

The implementation takes the filename of the configuration file from the
environment variable INTELMQ_MANAGER_CONFIG, just like
intelmq_manager.serve does.
 * Switch to python from php for CI.
 * Chance version number to be python setuputils compatible and silence
   a CI warning.
 * Remove outdated auth section in docs/INSTALL.md, add TODO instead.
@bernhardreiter
Copy link
Contributor

Some tasks left:

  • enable running the tests
  • enable running the type checks
  • document how to add and remove users
  • improve frontend for better handling if not-logged in (bring up login-dialog)
  • Do some more testing in error cases

 * remove php requirement, add python.
   Using explicit package name, until we change to using a
   requirements.txt file.
 * Add X-Python3-Version to control and add more build-dependencies.
 * Try bionic and using apt for getting pytho3 an packages.
 * change travis config to have access to side packages
@ghost
Copy link

ghost commented Aug 4, 2020

That looks like lots of efforts have been made to accomplish this! Many thanks!

Is this intended to get merged or is it WIP?

@bernhardreiter
Copy link
Contributor

Is this intended to get merged or is it WIP?

Both! ;)

Because there are some things which would be better, and it is likely that we'll improve them. On the other hand, it already improves an important security property, so it is a lot better than before and should see production as soon as possible.

This configuration reflects the command I've been using during
development.
@ghost ghost added this to the 2.3.0 milestone Oct 16, 2020
@ghost ghost self-assigned this Oct 16, 2020
@ghost ghost added the backend label Oct 16, 2020
@ghost ghost merged commit cebaef8 into certtools:develop Oct 16, 2020
@bernhardreiter bernhardreiter deleted the dev-csrf branch October 19, 2020 08:13
@bernhardreiter
Copy link
Contributor

Thanks @wagner-certat

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants