AWS Cognito provider #6917
AWS Cognito provider #6917
Conversation
related to issue #6822
Those instructions are based on experience with AWS Cognito but basically can work with any IDP provider
Codecov Report
@@ Coverage Diff @@
## master #6917 +/- ##
==========================================
+ Coverage 32.96% 33.03% +0.06%
==========================================
Files 1197 1219 +22
Lines 13020 13562 +542
Branches 1286 1348 +62
==========================================
+ Hits 4292 4480 +188
- Misses 7885 8200 +315
- Partials 843 882 +39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hi @ralphsomeday, this configuration should be added to the users-permission plugin config only not at the root of a strapi project config :)
alexandrebodin
added
source: plugin:users-permissions
|
Closing for inactivity @ralphsomeday Let me know when you can takle this again so I reopen it ;) |
|
Hi, I am new to strapi (just evaluating) and would need to integrate AWS Cognito user-pool as its auth provider (is it possible) but the documentation is minimum. @ralphsomeday is your PR for cognito integration? Would you please share any resources you use to get it up? |
|
@JiboStore i will review the comments from @alexandrebodin and update the PR. |
|
@alexandrebodin can you re-open it? |
[strapi] pass the subdomain from grantConfig
… than the aws cognito one
|
@alexandrebodin how can we resolve the merge conflicts with Travis? thanks |
|
@alexandrebodin would you mind reviewing this PR? this will allow to add support for a new provider AWS Cognito. |
There was a problem hiding this comment.
Hi, thanks for this PR. Using extra config folder is not pactical as it requires more user work. I think this should be set in the plugins.js users-permissions configuration. We could do it a better cleaner buy allowing to overxwritte any provider config from the configuraation directly like so:
./config/plugins.js
module.exports = () => ({
users-permissions: {
grantProviders: {
cognito: { // or any other provider
subdomain: 'azdaz',
}
}
}
})This way we only merge the configs together instead of having to had code for each new properties that will come up in the future
…in field in the configuration popup of the cognito provider so that the user can configure the subdomain directly in the admin panel UI. Added the field translation key in all the languages supported by the users-permissions plugin.
|
@alexandrebodin made it even easier by adding an extra field in the provider configuration and added a translation key in all supported languages see below screenshot:
|
|
@alexandrebodin any chance this can be reviewed so that we can add support for cognito auth provider in next release? |
|
Hi, your PR contains a ton of conflicts right now. Once it is fixed we will review it again. From What I saw already adding the input in the UI won't scale as we can't just add every possible input one PR at a time. I think we should still allow passing those configuration in the configs instead of the UI. |
|
@alexandrebodin the conflicts are only one translation key added ("PopUpForm.Providers.subdomain.label": "Host URI (Subdomain)") in the translation files, the auth file has been cleaned up to remove the changes and go back to its original shape and the bootstrap has just added the new AWS Cognito provider. I do not see why this won't scale at UI level, any new provider you are adding is going in the bootstrap and this is a new provider that is now working which allows companies using cognito as an idp to be able to connect it with strapi. |
|
I wasn't clear. I meant we won't be able to add input for every single options avaialbe for each providers. In this case I only see in the diff that you added the cognito provider. no issue with that but the PR is just not mergeable so we won't approve it until cleaned up :) |
|
@alexandrebodin the thing is that the subdomain is not a custom property of cognito it is provided with grant and can be configured for some providers that needs it. So i guess this is something we can leave in the UI. For the rest let me know what you would like me to have cleaned up so that this PR can go ahead? The subdomain in the UI will allow to configure different providers that may require different subdomains directly in the UI way more handy than having it in the configuration of the plugin. |
|
You need to fix all the conflicts so the PR can go ahead |
|
@alexandrebodin and @derrickmehaffy doc is updated please review and let me know if anything need to be amended or added. |
There was a problem hiding this comment.
Hi,
Thanks for you contribution !
It doesn't work on my side, I've been through the whole process but at the end I have "unknown provider". I think some code is missing in the PR here: https://github.com/strapi/strapi/blob/master/packages/strapi-plugin-users-permissions/services/Providers.js#L452
|
As @petersg83 mentionned you need to add a handler once you get the access_token back from aws cognito and call the callback with the right user infos :). |
|
Yes I am on it |
|
FYI, when you customize the client redirect URI you don't have that issue as you handle the access_token directly in your front-end app |
|
Yes that is what I assumed. The access_token will be sent to frontend and then they can handle the api calls from the front end. I am now getting a 431 Request Header Fields Too Large as the access token from Cognito is quite Large more than 7K characters. How would you deal with this limitation on the server side @alexandrebodin ? |
|
When are you getting this error exactly ? |
|
When passing the access token to the connect endpoint via the query parameter |
|
Hum I don't get an error. do you run this behind a proxy ? |
|
Can you share you configuration for the provider? |
|
For information, I tested the PR with this front-end example : https://github.com/strapi/strapi-examples/tree/master/login-react. I just added |
…ito + extending the scope of cognito so that we retrieve the id_token from cognito that will contain the information to setup a strapi user account and return a strapi jwt token to the frontend app
|
Thanks @petersg83 |
| @@ -161,6 +162,23 @@ const getProfile = async (provider, query, callback) => { | |||
| }); | |||
| break; | |||
| } | |||
| case 'cognito': { | |||
| // get the id_token | |||
| const idToken = query.id_token; | |||
There was a problem hiding this comment.
id_token doesn't seem to exist. In the query to auth/cognito/callback I see the params access_token, refresh_token, raw[access_token], raw[refresh_token], raw[expires_in] and raw[token_type].
Also, to do like the other providers, the code here is supposed to make a call to the cognito API to retrieve the username.
Does it work on your side with this front-end app: https://github.com/strapi/strapi-examples/tree/master/login-react ?
There was a problem hiding this comment.
you have to empty your core_store from the grant permissions as I had to update the provider oauth scope in the bootstrap file in order to retrieve the user profile information. So instead of only email we now retrieve the openid and profile scopes and you will now receive the id_token back from cognito.
There was a problem hiding this comment.
Thank, you are right and it works great with the front-end app now !
There was a problem hiding this comment.
@petersg83 and @alexandrebodin can we merge?
There was a problem hiding this comment.
Yes, it's good for me :) Just waiting for this: #6917 (comment)
There was a problem hiding this comment.
@ralphsomeday Error still persists, version of Node that I am currently using is v12.16.1. I think I should update my Node version to > 14 in order to be able to set --max-http-header-size=16384
There was a problem hiding this comment.
@ralphsomeday Error still persists, version of Node that I am currently using is v12.16.1. I think I should update my Node version to > 14 in order to be able to set --max-http-header-size=16384
Now it works 👍
There was a problem hiding this comment.
@ralphsomeday Sorry for taking this conversation in here, if it's not a good place to post this please tell me and I will delete this comment.
I just wanted to ask, would it be possible to implement this whole flow ( the one that's implemented by react-login example ) successfully while avoiding Cognito's Hosted UI, and instead using my own custom built UI ( that I've implemented by using SDK, which enables me to authenticate with Cognito without Hosted UI )? What would be the additional steps needed to be taken for Strapi's authentication part?
Once again, sorry for opening this topic in here, sources are scarce.
There was a problem hiding this comment.
Good question! You will have to research this one as usually the flow allows you to convert a cognito token into a strapi token which you then use to call the strapi api endpoints. If you authenticate using amplify or the sdk you will get a cognito token that won't be recognised by strapi. Maybe after getting the cognito token, you can call the redirect url to of the cognito provider with the access_token?
There was a problem hiding this comment.
Hm, sounds like an idea :) If I try and when I try I will definitely write about it. Thanks :)
Updating the redirect URL to your front-end app when using react-login or own frontend
petersg83
added
issue: enhancement
Description of what you did:
fix #6822