Oauth2 support for release manager #498
Conversation
| @@ -24,12 +25,26 @@ func pushCommand(options *Options) *cobra.Command { | |||
| var err error | |||
| ctx := context.Background() | |||
|
|
|||
| if releaseManagerClient.Metadata.AuthToken != "" { | |||
There was a problem hiding this comment.
There is a subtle change in behavior here, right?
Before: if no token is supplied it becomes a no-op. Now it will error out?
Not sure if that is important though, just looks like it.
There was a problem hiding this comment.
It is, i cannot see the reason for it not being set. Please enlighten me on the usecase 🙏
There was a problem hiding this comment.
@lunarway/squad-aura what do you think about this?
Its a matter of changing the middleware and provide the idp config on startup. Seems pretty simple, should i take a stab at adding that part and Remote the ham token all togethers? |
|
Depends on how much it ends up to be I guess. Aura aren’t super comfortable with the codebase but if it isn’t a large change it might be simple enough to incorporate🙏 |
hoeg
changed the title
|
implemented token verification in the server as well instead of relying on a proxy for handling the token. I have a todo to look at the file permissions of the token written to disk. |
I have not had the time to set it up locally. Please go for it if you have the time, if not then I will get to it later this week. Also, if you find anything during your test you are of course more then welcome to push changes to this branch 🙏 |
|
Converted back to draft to ensure we iron the last parts before Aura takes a look |
|
@Crevil I think we have been through all the comments now. I guess we just have to test the daemon and artifact cli's before going out of draft, right? |
|
Should we initiate a login if the token is invalid or should we just fail and tell the user to do a login instead? Doing automatic logins will hopefully make for at better user experience but that might break e.g. scripts that cannot pop a browser. Currently we just fail if the token is invalid and tell the user to log in. |
|
Guess we are ready for review now @Crevil? |
|
Yeah, I think so. Should we do a fresh PR to reduce the overhead of all our previous communication? |
|
Sure, I will close this PR and create a new one and make sure to add the outstanding questions to the description 🚀 |
This PR contains the introduction of the
logincommand tohamctlwhich will initiate an oauth2 device authorization flow.The access token is persisted to disk and when it is expired the user must do a login again.
The release daemon and artifact executables has been moved to client credentials flow to have oauth2 on all the calls.
We support both the old auth token that is shared as well as the new jwt. The server must be configured with an IdP but this version is kept backwards compatible such that old clients can still use the server. When users have updated their cli's then we can remove the support for the static token.
The changes include that:
$HOME/.Configdirectory (should be ok on all OS, right?)golang.org/x/oauth2packageActoris taken from the context, in case of the jwt supplied for auth or from the request if the old static access token is used.We have been holding back on documentation as we would like to agree to the changes before writing it 🚀