Skip to content
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

authentication design document. #65

Closed
wants to merge 2 commits into from
Closed

Conversation

bmccandless
Copy link
Contributor

This version of the document is a translation from the google doc version.
Some light reformattingh has been applied, but the content is basically identical.
I will followup with content changes to reflect the current state, and add
sections providing more details about the deployments.

@bmccandless bmccandless requested review from patangay, sbeara and a team as code owners October 1, 2020 22:58
rfcs/0001-authentication/text.md Show resolved Hide resolved
that lack a token or have an invalid/expired token. The signature needs to be
validated to ensure that the token has not been tampered with. An expired ID
token can be refreshed (as long as the refresh token has not expired).
1. [CXG7] The cellxgene server will have an endpoint to check if the requestor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a note here to explain under what user story, this endpoint will be hit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cellxgene frontend needs to know if the user is authenticated in order to show an Log In button or a Log Out button. I can add that. I have a new endpoint for this called "userinfo".

1. [CXG8] To avoid causing the ID token to expire while the user is working
(and thus disrupting the user’s work), the token can be automatically refreshed
when the ID token expires. The
expiration date of the Refresh token could be set to the maximum of 30 days.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this 90 days now? I forget...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this should be indefinite now. The refresh token is itself is refreshed when the id token expires. So, a user can be logged in indefinitely as long as they visit the website at least once per 90 days. I can add a comment for that.


1. [C1] An authenticated user will have the option to logout
1. [C2] An unauthenticated user will have an option to login

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there additional requirements that were met here that mirror the cellxgene requirements? For example, I'm thinking that number 8 (refresh cycle), number 6 (ensuring that token is clean), and number 7 might have already been implemented? Also a requirement that auth is hidden under a flag until it is ready to be turned on (or however this is being done).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, portal and cellxgene work in the same way. These should be reframed as common requirements.


General

1. [G1] GDPR requirements: For gdpr, the main thing we need to log is user id + time stamp for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious - where do we store this information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auth0 stores this. We can get the logs or access to the their DB if we need to.

rfcs/0001-authentication/text.md Show resolved Hide resolved
implemented in the client side. This has now been changed to
using the Authentication Code flow in the backend (same as cellxgene). This approach is
simpler with respects to the single sign on strategy. It avoids a step where the client
needs to access the server to set the cookie containing the tokens. It is also more secure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it more secure?


- Login: start the authentication flow, set the cookie when complete
- Logout: logout, remove the cookie
- Is-authenticated: check if authenticated, and return user information
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I can't remember if this was built or not, but can you replace with the actual endpoint name if it was?

that this question has already been asked, and we first verify the answer before proceeding
with the login process.

## Detailed Design | Architecture | Implementation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I forget, did we add any tables to the RDS for cellxgene to keep track of users? Or do we only store their identify for annotations?

cellxgene server in the diagram.

![Authentication Code Flow](imgs/authcodeflow.png)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're willing/able to, I think a sequence UML diagram that showcases the steps for the login flow you've described below would be super helpful! It would show a clear picture of when the endpoints are called, by whom, and in what order. You could even substitute it for the above diagram which I (personally) don't find to be super helpful without cross checking with the wording below.

Brian McCandless and others added 2 commits October 6, 2020 09:03
This version of the document is a translation from the google doc version.
Some light reformattingh has been applied, but the content is basically identical.
I will followup with content changes to reflect the current state, and add
sections providing more details about the deployments.

## References

[0][location to the authorization code flow diagram](https://auth0.com/docs/architecture-scenarios/web-app-sso/part-1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmccandless if we want the link text not be lowercase, we'll need to add \ before the number like below, otherwise linter will force lowercase! Thank you!

## References
\[0][IETF's Github Process](https://tools.ietf.org/html/draft-ietf-git-using-github-06)
\[1][Google Developer Documentation Style Guide](https://developers.google.com/style/highlights#introduction_1)
\[2][Data Coordination Platform RFC Process](https://github.com/HumanCellAtlas/dcp-community/blob/master/rfcs/text/0001-rfc-process.md)


- The login request. The client sends a request to the “/login” endpoint of the cellxgene
server. The request contains the location to the current dataset which will be used to
redirect the user back at the end of the process. The location can be path within the current
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
redirect the user back at the end of the process. The location can be path within the current
redirect the user back at the end of the process. The location must be a path within the current

- The cellxgene server then redirects (full page nav) the user back to the original dataset
, and directs the client to save the JWT in an httpOnly cookie on the client.
- The JWT proves the identity of the client. Subsequent requests from the client to the
cellxgene server will include the cookie with the JWT. When the cellxgene client re-requests
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cellxgene server will include the cookie with the JWT. When the cellxgene client re-requests
cellxgene server will include the cookie with the `id_token`. When the cellxgene client re-requests

might be worth the extra specificity wrt the oauth2/oidc protocol

process by going back to step [1]
2. The server will verify the signature of the token to ensure that it was signed by the
sender and not altered.
3. If the ID token is expired, then the server will obtain a new ID token automatically
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
3. If the ID token is expired, then the server will obtain a new ID token automatically
3. If the ID token is expired, then the client will obtain a new ID token automatically

I think this happens client-side?


An alternative is to use the authorization code with proof key code exchange (PKCE). This is
the approach you might use in a single page app where you do not want the code running in a user’s
browser to have access to the auth0 client secret.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
browser to have access to the auth0 client secret.
browser to have access to the oauth2 client secret.

@Bento007
Copy link
Contributor

Bento007 commented Nov 3, 2020

@bmccandless bmccandless deleted the bmccandless/auth branch January 14, 2021 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants