ktra icon indicating copy to clipboard operation
ktra copied to clipboard

Add support for openid for authentication

Open gagbo opened this issue 2 years ago • 5 comments

TODO

  • [x] Update the readme with the new image tags.
  • [x] Make a PR to https://github.com/moriturus/ktra-book to add the oid guide, and explicitely state somewhere that a user can currently only have one active and valid token at any given time, just like the other implementations.
  • [x] Add another oid endpoint to only fetch the current token without changing it.

PR Summary

The implementation is being dogfed with gitlab as the OpenID issuer.

In order to work, the issuer needs to:

  • be discoverable (with GET /.well-known/openid-configuration)
  • have a userinfo_endpoint in the openid configuration

Authentication is done through white-listing. If there are no whitelist fields, then anyone can create an account, therefore can publish and own crates in the registry.

When ktra is built with openid, all the user management endpoints are disabled to avoid tampering through unauthenticated POST calls.

Also, there is no point storing a password, but as the password interface is strongly coupled with the DbManager trait, for the time being a dummy password is inserted for users. This is deemed not dangerous as all authenticated routes aren't compiled when the "openid" feature is present

A user_by_login function has been added to the DbManager trait because the login is now dynamically computed from the OpenId issuer.


Other notable changes

  • A test workflow, mostly to test all db implementations with/without openid (it should at least compile)
  • Bumping the docker base image to Rust 1.56, latest stable version
  • Push extra docker images with openid enabled

Notes

I tried to keep the code somewhat generic in order to allow other issuers to plug in easily, but the changes are still geared towards my company's usecase (being "we want to use gitlab openid for user authentication"). This shows mostly in 4 different points :

  • requiring a discoverable config
  • requiring a userinfo_endpoint
  • adding extra fields in models::Claims which (optionally) match the extra claims from gitlab OpenId I want to use
  • adding extra fields in config::OpenIdConfig which (optionally) describe the authorized gitlab entities on the registry

As is, I think it's ready for this usecase, so I'm pushing the PR here if anyone wants to review it/start using it, modifications are welcome. (I know I can build a docker image from it for my use case so that's good enough for me even if the PR doesn't make it)

Example

With this config

[index_config]
remote_url = "[redacted]"
https_username = "gagbo"
https_password = "[redacted]"
branch = "master"

[openid_config]
issuer_url = "https://gitlab.com"
redirect_url = "http://registry.szl"
client_id = "717dc...."
client_secret = "[redacted]"
additional_scopes = ["openid", "profile", "email"]
gitlab_authorized_groups = [ "firm/groups/crate_owners" ]
gitlab_authorized_users = [ "gagbo" ]

[server_config]
port = 80

The client_id/client_secret pair is coming from Gitlab where I added an application to my profile for test purpose, with the scopes openid, profile, and email

ktra-test-oauth-app

Upon going to registry-url/me I arrive at the sign on for Gitlab where I authorize the app, and then after redirection firefox is showing me my token :

post_gitlab_signon

(The username has since been modified to gitlab.com:gagbo instead)

and I can use it locally :

cargo_login_publish

gagbo avatar Nov 18 '21 12:11 gagbo

@gagbo, I have been setting up a ktra server using this PR, and it is very pleasant to use. Thank you!

Do you have plans to enable adding/deleting users from API endpoints instead of defining the authorized users/groups directly from the config file/CLI options?

jbeaurivage avatar Mar 01 '22 22:03 jbeaurivage

I recently got invited to the organisation to get this merged first with the few other changes that were pending, I'll need to find time to finish this first.

Currently I don't plan to add extra endpoints to deal with it, but PRs are of course welcome. I suppose the main point here would be to actually use the db_manager to add extra functions related to permission handling, it was easier for me as a first PR to just stuff everything in the config file. I'll try to take a look into this, but I don't expect to have much time for ktra features before ~2 months I think

Glad you like it :) I've been using this as well for our startup and it definitely fits the bill

gagbo avatar Mar 01 '22 22:03 gagbo

Oh absolutely no problem, I was just throwing ideas out there. As I use ktra I might start contributing as well :)

jbeaurivage avatar Mar 01 '22 23:03 jbeaurivage

I changed the base branch of this PR to develop.
So could you also rebase your working branch?

moriturus avatar Mar 25 '22 15:03 moriturus

I changed the base branch of this PR to develop. So could you also rebase your working branch?

I'll do that yes. I will need to test this PR a bit more locally as well. I understood recently that a user is only allowed to have a single token in the database, so my current implementation always silently revoked the old token whenever a user went to the /me url. I'm changing the /me endpoint to return the existing token if it exists, and adding a /replace_token endpoint to forcefully rotate the token (in case the user leaked the token in a CI pipeline or somewhere else, and wants to revoke it)

gagbo avatar Mar 25 '22 17:03 gagbo