-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add API endpoint to get oauth request/consent #5699
Conversation
# OAuth | ||
OAuthConsentRead: | ||
type: object | ||
required: | ||
- consentUrl | ||
properties: | ||
consentUrl: | ||
type: string |
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.
BTW, should the OAuthConsentRead
returned with the connector's definition id that was provided as input too?
In that case, we'd have to have a OAuthSourceConsentRead
& OAuthDestinationConsentRead
?
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 don't think so. The UI has access to that information when it receives this response.
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 LGTM but we need @jamakase's review. Adding him here.
@@ -1182,6 +1184,52 @@ paths: | |||
$ref: "#/components/responses/NotFoundResponse" | |||
"422": | |||
$ref: "#/components/responses/InvalidInputResponse" | |||
/v1/source_oauths/get_oauth_consent: | |||
post: |
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.
it feels like this should be a GET so the server redirects the user directly, but it would break our API convention. With this being a post, the UI would need to redirect the user. NBD I think.
# OAuth | ||
OAuthConsentRead: | ||
type: object | ||
required: | ||
- consentUrl | ||
properties: | ||
consentUrl: | ||
type: string |
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 don't think so. The UI has access to that information when it receives this response.
…e into chris/oauth-request-api
What
Closes #5639 and #5640
The endpoints in this PR will be leveraged as part of this larger flow
Recommended reading order
config.yaml
ConfigurationApi.java