-
Notifications
You must be signed in to change notification settings - Fork 0
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
[ID-1051] Add GitHub account linking #165
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try and use the Provider
enum as much as we can in the codebase. Passing magic strings around is bound to lead to confusion if we keep adding providers.
} | ||
|
||
public Optional<LinkedAccount> createLink( | ||
String providerName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a Provider
, not a String
. That way, we can do validation on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change it to be consistent with #168. I'm not sure that we need to check if this is a valid provider here since that already happens upstream. Unless you mean validating that it is a token-based provider?
@@ -76,6 +80,7 @@ public ResponseEntity<LinkInfo> createLink( | |||
.userId(samUser.getSubjectId()) | |||
.clientIP(request.getRemoteAddr()); | |||
|
|||
Optional<LinkInfo> linkInfo; | |||
try { | |||
if (providerName.equals(Provider.RAS)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since providerName
is an enum value, we can replace this if/else
with a switch
statement
Jira:
What:
Add
github
as a provider option for the following endpoints:/api/oidc/v1/{provider}
/api/oidc/v1/{provider}
/api/oidc/v1/{provider}/authorization-url
/api/oidc/v1/{provider}/oauthcode
The only endpoint changed in this PR is the POST endpoint. If the provider is
github
, the createLink method for token providers will be called (not passport providers).Why:
This PR enables a user to link their github account, which will allow them to import private workflows into their Terra workspace.
How:
TokenProviderService
with acreateLink
method that handles account linking for any provider that uses access tokens.Note: This is blocked by https://broadworkbench.atlassian.net/browse/ID-1044 (adding github to the ECM provider configuration)