Public Authentication Implementation #20
Conversation
| let response = client | ||
| .get("https://api.github.com/user") | ||
| .header("accept", "application/json") | ||
| .header("user-agent", "wally") |
There was a problem hiding this comment.
Should this be UpliftGames/wally? I vaguely remember GitHub requesting that you do that.
There was a problem hiding this comment.
If I understand the documentation correctly then this is our oauth application name, not our repo name.
| &self.github | ||
| } | ||
|
|
||
| pub fn can_write_package( |
There was a problem hiding this comment.
I think this is a little more than can_write_package because it also mutates the index if the user can write. In the future, we might have an API endpoint for the frontend where we might want to just check if the user can write.
Additionally, can_do_x would usually return a bool! This is more like... "let me write to this package"
What's a better name for this method?
There was a problem hiding this comment.
This is a good point! I've kept the name and changed this to return a bool for "if the user should be allowed to write a package". Ie they can have permission but not be an owner. We therefore then later check to see if they are an owner and if they aren't we add them as we know they have authorization to write. This seems to make far more semantic sense!
wally-registry-backend/src/auth.rs
Outdated
| AuthMode::ApiKey(key) => match_api_key(request, key, Self { github: None }), | ||
| AuthMode::DoubleApiKey { write, .. } => { | ||
| match_api_key(request, write, Self { _dummy: 0 }) | ||
| match_api_key(request, write, Self { github: None }) | ||
| } | ||
| AuthMode::GithubOAuth => verify_github_token(request).await, |
There was a problem hiding this comment.
Having github: None in all non-GitHub auth cases is a little funny. I think we've got a couple options to make this struct fit our intent a little better.
I think my favorite is to make WriteAccess an enum:
pub enum WriteAccess {
/// Granted from a global API key; the user can do whatever they want!
Global,
/// Granted from a GitHub login; the user has limited scope
GitHub(GitHubInfo)
}This makes our matches a little more semantically clear, as a nice bonus.
There was a problem hiding this comment.
I forget how nice Rust enums are! I've used Global to indicate everyone has this access and ApiKey to indicate if they have access because of an api key. Having Global vs ApiKey doesn't make any functional difference atm but does add a lot of meaning, for example WriteAccess doesn't have a Global as we intentionally don't support that. Doing this has really cleaned up stuff semantically and it's all feeling incredibly clean!
There was a problem hiding this comment.
Actually I've decided to use Public instead of Global. I realised you probably meant global as in "access to all packages" so my usage could be confusing.
This PR is the implementation of public authentication! It closes issue #8, see that for more info.
This PR:
wally loginto ask for an api key or present an oauth flow if the registry has an oauth client idowners.jsonfiles in scopes to track ownership and enable ownership by multiple users (although no command to do so exists yet)