awslabs / service-workbench-on-aws

A platform that provides researchers with one-click access to collaborative workspace environments operating across teams, universities, and datasets while enabling university IT stakeholders to manage, monitor, and control spending, apply security best practices, and comply with corporate governance.
Apache License 2.0
178 stars 119 forks source link

Authentication Provider broken after upgrade to v3.0.0 #469

Closed primerano closed 3 years ago

primerano commented 3 years ago

We had authentication via Okta/cognito working in V1 of SWB but after upgrading it appears things are now broken. This is a single hosting account setup.

Before the upgrade we were on 0d819ac913327ab36a30d93c4dd12487c152842d

I checked out v2 and did a deploy and then v3

git checkout tags/v2.0.0
scripts/environment-deploy.sh $STAGE_NAME
git checkout tags/v3.0.0
scripts/environment-deploy.sh $STAGE_NAME

At this point I tried signing in and I get the following error in the URL

error_description=Error+in+SAML+response+processing%3A+Invalid+ProviderName%2FUsername+combination.+Please+check+again+&error=server_error

If I log in as root and go to Users I noticed that the Identity Provider has been capitalized so the user object is pointing to a non-existent IP.

ex: my Identity Provider Name is okta but in the the User's listing it shows as Okta and if I try to edit the user IP Name select box is empty since Okta doesn't match okta. If I select the lowercase version from the select statement I get validation errors on save.

{"requestId":"","code":"badRequest","message":"Input has validation errors","payload":{"validationErrors":[{"keyword":"additionalProperties","dataPath":"","schemaPath":"#/additionalProperties","params":{"additionalProperty":"uid"},"message":"should NOT have additional properties"}]}}

I can delete the user and start over but I get the same error

#error_description=Error+in+SAML+response+processing%3A+Invalid+ProviderName%2FUsername+combination.+Please+check+again+&error=server_error

I'm not sure how to debug this. Any help would be appreciated.

jn1119 commented 3 years ago

Hi Tony, I have started looking into this issue. Can you let me know if you are blocked or are able to use a prior commit 0d819ac to get around the issue?

Thanks, Jeet

jn1119 commented 3 years ago

Also, the fedIdpName is taken from your stage.yml file under main/config/settings directory which is specified for key fedIdpNames. Did you change that before deployment?

jn1119 commented 3 years ago

I would also recommend you to look at Attribute Mapping in your Cognito UserPool and verify that your SAML settings are correct. You can verify the IDP name there too and check if SAML attribute and User pool attributes are correctly matched.

As a related question, I asked before if fedIdpNames in stage.yml change but can you also take a look if other attributes like SAML metadata file or fedIdpIds change also change?

primerano commented 3 years ago

@jn1119 I think it took us a few tries to get okta working so I have an Okta and okta IdPs setup in cognito.

In our stage yml file we have the lowercase version specified but I think the Okta version is what worked. Sorry, its been over a month since we did this. I edited the okta version to have the same mappings as Okta but I still get the same error.

I tried switching to the capitalized version in my stage yml file but the deploy failed as the IdP Identifier was already associated with the lowercase version.

"message": "IdP Identifier mysubdomain.oktamydomain.com is already associated with a different identity provider okta",

I have not tried reverting to the prior commit as I wasn't sure if that would break things even further. I reverted and I get the same error.

How do I make the okta version go away so I can use the Okta IdP. Are there debug logs somewhere I can look at?

Sorry, this is a mess now. :-\ Tony

Going back to v3.0.0 i get an error about

primerano commented 3 years ago

v3.0.0 now gives me...

An error occurred: StudiesDb - Attempting to create an index which already exists (Service: AmazonDynamoDBv2; Status Code: 400; Error Code: Vali
dationException; Request ID: TTUPM7D7KTSL77PUKQAKUK87HJVV4KQNSO5AEMVJF66Q9ASUAAJG; Proxy: null).

I think at this point I may just need to start over

jn1119 commented 3 years ago

For the error you are facing when going from 2.0 to 3.0, it seems like you would need to delete AccountIdIndex in your Studies table. The issue is that this index was created for you in 3.0 but it wasn't deleted in 2.0 for some reason. Might be a separate CFN issue. And now when you are installing 3.0, the index is being re-created but since it already exists you are seeing an error.

image

jn1119 commented 3 years ago

Can you elaborate the difference between Okta version and Okta IdP. I am not that familiar with Okta but I have begun setting it up so that I can reproduce the issue and understand it better. If there are two SAML providers, have you tried deleting the unused one from Cognito?

primerano commented 3 years ago

Deleting that index and a log group allowed me to successfully install 3.0 again.

I put the lowercase and capitalized IdP name in my stage.yml so that I they will both appear in the User Detail IdP dropdown. If I try to save the user am still getting an error about uid not being allowed. Is this something that changed in this form?

{"requestId":"", "code":"badRequest", "message":"Input has validation errors", "payload":{"validationErrors": [{"keyword":"additionalProperties","dataPath":"","schemaPath":"#/additionalProperties","params": {"additionalProperty":"uid"},"message":"should NOT have additional properties"}]}}

I also think i may understand why my SAML login broke. We had manually edited the Cognito Federation Attribute Mappings for Okta to work.

http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name -> Name was changed to name -> Name

It looks like these settings are changed back when I do my deploy so I need to manually correct the mappings. Now my login works. I do have 2 identity providers now. How do I delete one and point all users to the other one? As noted, if I try to update a user in swb I get that uid error.

primerano commented 3 years ago

to get around the uid error i deleted the users that were pointing to the IdP I deleted and recreated them under the remaining IdP.

2 items to address.

jn1119 commented 3 years ago

Are you changing the user's idp from empty to some idp? An empty identity provider usually indicates internal authentication and you won't be able to go back and forth since that changes the original identity of the user. In any case, I don't think we internally from SWB support changing identity providers. There can be a way to do that from internally managed DynamoDB table directly but I would recommend you to create those new users if there aren't many. I hope that answers the question of why IdP cannot be changed since that changes the uniqueness of the user (since IdP is one of the attributes that uniquely identifies a user)

image

image

jn1119 commented 3 years ago

I don't think Cognito mappings should be overwritten by new deployment. From this linked code it seems like we are specifying the following user mappings in code:

  1. name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name'
  2. given_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname'
  3. family_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname'
  4. email: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'

So for the configured IdP in your stage file, I would expect Cognito mappings to be what is specified in code. Can you double check if a deployment changes the expected config 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name' to name for the configured IdP in your stage file

primerano commented 3 years ago

"Are you changing the user's idp from empty to some idp?" - no i was trying to change from my Okta IdP to my okta Idp.

With my Okta settings I need the following mapping (the last mapping is not needed, i was experimenting)

okta_mappings

after deployment they are changed to

swb_mappings
jn1119 commented 3 years ago

Yeah, based on the code I linked it seems like it is expected for SAML attributes to have the prefix http://schemas.xmlsoap.org/ws/2005/05/identity/claims/ after deployment. Is it possible for you to change the attributes in your Okta Idp configuration.

Edit: So in your Okta IdP configuration can you modify the SAML attributes to match what you see in Cognito after deployment?

primerano commented 3 years ago

I guess this is possible but since my IT department controls that I will continue to update these fields after deployment. If an Okta guide is added in the future that might be helpful.

I think we loosely followed the auth0 instructions. http://swb-documentation.s3-website-us-east-1.amazonaws.com/deployment/configuration/auth/configuring_auth0

jn1119 commented 3 years ago

So action item on our side would be to allow custom mappings. Currently it seems like we hardcode them to have prefix http://schemas.xmlsoap.org/ws/2005/05/identity/claims/. Is that the correct action item?

primerano commented 3 years ago

your prefix seems reasonable. You mentioned that you were trying out Okta. If you can get it working with that prefix there is probably no reason to make it configurable. An Okta setup guide would be helpful.

If it is easy to make it configurable in the stage yml file that may come in handy.

jn1119 commented 3 years ago

We haven't been able to prioritize Okta integration and validation of those fields yet. We will be looking into validating those fields this week or next week, and come up with next steps and guidance. But do let us know if the issue is blocking you in some way. Thanks.

dragan777 commented 3 years ago

Hello, any update on this issue. We are facing the same problem and it happens only for users who are linked with oidc provider configured in cognito. This is not the case for linked Google and Facebook users.

jn1119 commented 3 years ago

@dragan777 can you elaborate the issue you are facing? In this particular issue we were exploring an option of allowing customers to provide their own custom prefix for attributes. Currently the prefix for all the attributes is http://schemas.xmlsoap.org/ws/2005/05/identity/claims in SWB and there is no way to change that(unless you make a manual code change). Is that what you are facing?

dragan777 commented 3 years ago

@jn1119 sorry if i misunderstand this thread, we do not have a problem with a custom prefixes for attributes. Our problem is the sign in, when we try to sign in we are getting this error _error_description=Error+in+SAML+response+processing%3A+Invalid+ProviderName%2FUsername+combination.+Please+check+again+&error=servererror which is mentioned above. Feel free to remove the comment if they are not related to the thread

jn1119 commented 3 years ago

@dragan777 Thanks, and no worries. I would recommend that you create another Github issue and add more details by using the following template. Thanks