Hex OAuth #5324
Hex OAuth #5324
Conversation
|
Oops, this was meant to be a draft |
|
This is ready for initial reviews! |
| }, | ||
| }; | ||
| let toml = toml::to_string(&credentials).expect("OAuth credentials TOML encoding"); | ||
| crate::fs::write(&path, &toml)?; |
There was a problem hiding this comment.
This will create the file with the user's default umask, which may include it being world-readable. A best practice would be to set permissions to read-write for the user (0600 on Unix-like systems).
| crate::hex::HexAuthentication::new(&runtime, hex_config.clone()).get_or_create_api_key()?; | ||
| let credentials = crate::hex::HexAuthentication::new(&runtime, &http, hex_config.clone()) | ||
| .get_or_create_api_credentials()?; | ||
| let http = HttpClient::new(); |
There was a problem hiding this comment.
Is this still needed if there is one on line 26 now? The struct looks stateless.
| let (parts, body) = response.into_parts(); | ||
| match parts.status { | ||
| StatusCode::OK => (), | ||
| status => return Err(ApiError::unexpected_response(status, body)), |
There was a problem hiding this comment.
What happens if the token was already revoked server-side? I am guessing there would be a 404 here, and the flow would error out.
| fn default_poll_interval_seconds() -> u64 { | ||
| 5 | ||
| } | ||
| /// Use a refresh tokent to get an access token that can be used to |
| repository: http::Uri, | ||
| /// An encrypted refresh token. | ||
| refresh_token: String, | ||
| /// The hash of the token, so it can be revoked even if it cannot be ecrypted. |
There was a problem hiding this comment.
Typo in the word encrypted
| // session is which in the Hex console. | ||
| let client_name = format!( | ||
| "Gleam ({})", | ||
| hostname::get().expect("hostname").to_string_lossy() |
There was a problem hiding this comment.
Is it worth panicking if the hostname can't be found? Seems unlikely, but also easy to just fall back to "unknown."
|
|
||
| fn default_poll_interval_seconds() -> u64 { | ||
| 5 | ||
| } |
There was a problem hiding this comment.
Probably want a space between the curly and the doc comment for the next method.
There was a problem hiding this comment.
These changes seem unrelated to the rest of the PR, were these debugging changes left over?
|
|
||
| if let Some(key) = self.read_and_decrypt_stored_api_key()? { | ||
| return Ok(key.unencrypted); | ||
| if let Some(tokens) = self.read_and_decrypt_and_refresh_stored_tokens()? { |
There was a problem hiding this comment.
If the token can't be refreshed for any reason, such as being expired, I wouldn't expect this to propagate the error upward. It would be better to discard it and start the OAuth flow.
| /// In order, it will try these sources: | ||
| /// | ||
| /// 1. An API key from the HEXPM_API_KEY environment variable. | ||
| /// 2. An OAuth refresh token from the file system, which is the exchanged for an access token. |
There was a problem hiding this comment.
| /// 2. An OAuth refresh token from the file system, which is the exchanged for an access token. | |
| /// 2. An OAuth refresh token from the file system, which is then exchanged for an access token. |
3 participants
Moving from Hex basic-auth to OAuth.
This work is a collaboration with the Erlang Ecosystem Foundation.