-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat: Github SSO login #145
base: main
Are you sure you want to change the base?
Conversation
Related to fynnfluegge#74 Add Github login functionality to the application. * **cdk/rocketnotes.go** - Add a new lambda function for Github OAuth 2.0 App. - Add OpenID Identity Provider to Cognito UserPoolClient. - Update the stack to include the new lambda function. * **webapp/src/main.ts** - Add Github OAuth 2.0 App configuration to the `oauth` object. - Update `Auth.configure` to include Github as an identity provider. * **webapp/src/aws-exports.js** - Add Github OAuth 2.0 App configuration to the `awsmobile` object. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/fynnfluegge/rocketnotes/issues/74?shareId=XXXX-XXXX-XXXX-XXXX).
Thanks a lot @rohitg00 for this amazing PR! Is it still in progress or can I start adding review comments? |
Please add comments, it'll help me to understand your expectations. Thanks |
webapp/src/aws-exports.js
Outdated
"identityProviders": { | ||
"GitHub": { | ||
"client_id": "YOUR_GITHUB_CLIENT_ID", | ||
"client_secret": "YOUR_GITHUB_CLIENT_SECRET", |
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.
YOUR_GITHUB_CLIENT_SECRET
should not be exposed in the webapp
. This should rather be set as env
in a proxy-lambda. This proxy lambda should be responsible to authenticate to the Github OAuth Server. The proxy lambda can be configured in the Cognito UserPoolIdentityProvider
.
'client_secret': 'YOUR_GITHUB_CLIENT_SECRET', | ||
'authorize_scopes': 'read:user,user:email' | ||
} | ||
} |
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 am not sure if this is needed since it is already done in Amplify.configure(awsconfig);
. Correct me if I am wrong and this is doing different things.
Hi @rohitg00 I left some comments. In general this can be a good starting point. I think there is more work to do. That's why I put the label const idp = new cognito.CfnUserPoolIdentityProvider(
stack,
"GitHubIdentityProvider",
{
providerName: "GitHub",
providerType: "OIDC",
userPoolId: auth.userPoolId,
providerDetails: {
client_id: process.env.GITHUB_CLIENT_ID,
client_secret: process.env.GITHUB_CLIENT_SECRET,
attributes_request_method: "GET",
oidc_issuer: "https://github.com",
authorize_scopes: "openid user",
authorize_url: "https://github.com/login/oauth/authorize",
token_url: api.url + "/token",
attributes_url: api.url + "/user",
jwks_uri: api.url + "/token",
},
attributeMapping: {
email: "email",
name: "name",
picture: "avatar_url",
},
}
); The Then the final question to me is how to do the actual Login in the UI. I don't know if it is possible in the Cognito hosted UI similar as: Maybe this is working out of the box if correctly configured with IdeaSince Cognito offers built in support for Google SSO, what about reusing some code from here and adding Google SSO login? This is way easier and maybe worth some learnings before starting with the Github SSO which is a bit more complicated. Wdyt? |
Makes sense, I'll work next week on this and submit new PR. THANKS |
- Added secure token exchange Lambda - Added GitHub user info Lambda - Added user profile management with DynamoDB - Added logging middleware - Updated CDK stack with new resources - Removed client secret from frontend - Fixed Go module structure Closes fynnfluegge#74 Addresses security concerns from PR fynnfluegge#145
Added this issue #149 |
- Add Google OAuth Lambda handler with security features - Add OAuth callback component for handling redirects - Update CDK stack with OAuth configurations - Add security headers and CORS configuration - Add detailed OAuth setup documentation - Implement rate limiting structure - Add token validation structure Resolves fynnfluegge#149 Resolves fynnfluegge#145
Hi @rohitg00, thanks for submitting the changes! I will have a closer look soon |
"X-Amz-Date", | ||
"X-Api-Key", | ||
"X-Amz-Security-Token", | ||
), |
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.
Would be great if AllowOrigins
and AllowHeaders
can be done in a separate PR
const cognito_domain = `https://${environment.domainName}.auth.${environment.awsRegion}.amazoncognito.com`; | ||
const redirect_uri = encodeURIComponent(environment.redirectSignIn); | ||
const hosted_ui_url = `${cognito_domain}/login?response_type=code&client_id=${environment.cognitoAppClientId}&redirect_uri=${redirect_uri}`; | ||
window.location.assign(hosted_ui_url); |
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 think here is no change needed
|
||
// Add OpenID Identity Provider to Cognito UserPoolClient | ||
cognitoUserPoolClient := awscdk.NewCfnResource(stack, jsii.String("CognitoUserPoolClient"), &awscdk.CfnResourceProps{ | ||
Type: jsii.String("AWS::Cognito::UserPoolClient"), |
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.
Just saw it now, this will add a UserPoolClient
but what is needed is adding an IdentityProvider
to the UserPool
Hey @rohitg00 I added some comments. I would like to split Google and Github login in two separate PRs. As mentioned earlier the proxy auth endpoints are not needed with Cognito and Google SSO as this is supported out of the box in contrast to Github. But we can reuse some of your work in For now I propose to finalise the Google login here #152 Again, thanks for your work! |
Related to #74
Add Github login functionality to the application.
cdk/rocketnotes.go
webapp/src/main.ts
oauth
object.Auth.configure
to include Github as an identity provider.webapp/src/aws-exports.js
awsmobile
object.