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

Decision: choose an auth-n option for the website #117

Closed
ace-n opened this issue Jul 19, 2021 · 19 comments · Fixed by #139
Closed

Decision: choose an auth-n option for the website #117

ace-n opened this issue Jul 19, 2021 · 19 comments · Fixed by #139
Assignees
Labels
component: website Related to the application frontend. needs decision record This issue or change is significant and we should create a decision record. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Milestone

Comments

@ace-n
Copy link
Contributor

ace-n commented Jul 19, 2021

Summary

We should figure out how we're going to do the following:

a) determine whether a user is logged in
b) determine who a user is ("authentication")

Authentication Options

Username only

This option asks users to specify a username, and logs them in as that user. This model assumes no (presumably malicious) users will lie about their username.

✅ Easy to implement
✅ Easy to debug
❌ Doesn't work in production (insecure)

Username and password

This option asks users to specify a username along with a password. If the user forgets their password, they can request a Reset Password link to be sent to their email address.

✅ Works in production
❌ Lots of subcomponents to implement (such as password reset logic)
❌ Cryptographically securing password storage is somewhat easy to mess up
❌ May require infrequent future updates (if vulnerabilities are found)

Google Sign-In

This option asks users to sign into their Google account, and provides our application with a token corresponding to that specific user.

✅ Works in production
✅ No cryptography to mess up
❌ No existing knowledge-base on our team
❌ May require frequent future updates (depends on library update cadence)

GCP Identity platform

✅ Works in production
✅ No cryptography to mess up
❌ Small (if any) existing knowledge-base on our team
❌ May require frequent future updates (depends on library update cadence)


Recommendations

In this situation, Google sign-in or Identity Platform likely our best bets.

Google Sign-In

We can mitigate the risk posed by updates by introducing automated tests of the sign-{in, out} flows and updating these dependencies automatically (e.g. via Renovate).

Identity Platform

The following documentation pages may be helpful:

@ace-n ace-n added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. component: website Related to the application frontend. needs decision record This issue or change is significant and we should create a decision record. labels Jul 19, 2021
@ace-n ace-n self-assigned this Jul 19, 2021
@ace-n
Copy link
Contributor Author

ace-n commented Jul 19, 2021

cc @grayside @engelke as FYI

@engelke
Copy link
Contributor

engelke commented Jul 19, 2021

That first link seems to say users have to register for your app before they can sign in. Am I misunderstanding it?

I've used the Google Identity flow at https://developers.google.com/identity/protocols/oauth2/web-server which hasn't behaved that way. Any user with an account that Google can authenticate can use that to sign in. I recommend that approach.

@ace-n
Copy link
Contributor Author

ace-n commented Jul 20, 2021

@engelke SGTM.

The Google Identity docs mention session creation + tracking though (i.e. Google Identity does not handle this for us). Given that, should we:

  • generate a session ID on the backend, and store that in a cookie?
  • store the auth data itself in a cookie, and re-validate it each time?
  • do something else?

(Let me know if you think this should be discussed in its own issue, too.)

@grayside
Copy link
Collaborator

While we need to anticipate future needs, the initial auth implementation shouldn't significantly exceed what we need for managing donations. @averikitsch did research on Cloud Run + Identity Platform, would be interested in having her weigh in.

@grayside grayside added this to the v0.5.0 milestone Jul 20, 2021
@averikitsch
Copy link
Contributor

TL;DR: Identity Platform has a fuller set of features (extendability) with libraries that have a better devX. I believe it is more enterprise ready.

IdP can set up to have multiple providers ie Facebook, username/password, etc. It also has systems/functionality to manage users and user token states. The server and client side libraries for IdP are more readable. Both can generate tokens and both need to use client libraries to verify the identity. (note: Google Sign in tokens if the email is registered through IAM with invoker permissions can automatically be verified by a private service, but this is more of an antipattern). Both have some session management / auth persistence. IdP does have more guidance around sessions/cookies with built in functions compared to Google sign in. You should never store a token in a cookie but IdP can easily generate short lived tokens on the client side it usually will automatically log you back in and there is functionality built in for server side sessions. For IdP you shouldn't have to register the app before signing in.

@ace-n
Copy link
Contributor Author

ace-n commented Jul 20, 2021

FWIW - it seems like Identity Platform requires Firebase Auth, which may complicate our {setup, frontend installation} processes.

@averikitsch hopefully I'm mistaken here? I'm a bit wary of adding more dependencies than necessary, unless there's a compelling value-add.

@averikitsch
Copy link
Contributor

From offline convo: Client side libraries can be included via CDN, so no build system is needed:

<!-- Firebase App (the core Firebase SDK) is always required and must be listed first-->
<script src="https://www.gstatic.com/firebasejs/7.18/firebase-app.js"></script>
<!-- Add Firebase Auth service-->
<script src="https://www.gstatic.com/firebasejs/7.18/firebase-auth.js"></script>

Both need the GCP console to generate an OAuth 2.0 Client ID. No Firebase console is needed.

@engelke
Copy link
Contributor

engelke commented Jul 21, 2021 via email

@grayside
Copy link
Collaborator

grayside commented Jul 22, 2021

Suggest we try to be clear about Firebase Auth. We're talking about using the Firebase Auth client, which does not have any dependency on Firebase services: we'd use this as a library to interact with Cloud IdP. Anything we implement for auth will have some client-side component.

In looking at the options, I'm also curious:

  • Do they have significant setup latency? We have a requirement for 10 minutes for a new instance.
  • Do they require manual steps to set up? Each manual step complicates setup and any tutorials we create.
  • How do we support local development? Do we need to do something extra to run the app without auth enabled? (E.g., login buttons are no-op and link to pantheon to complete setup)
  • If there is a manual step, does it require redeploying our service?

@engelke
Copy link
Contributor

engelke commented Jul 22, 2021

I believe that using identity platform requires creating a project in order to get the necessary credentials for the app, regardless of whether it's Firebase or server-side. See https://github.com/engelke/google-signin-demo for a simple server-side use of identity platform.

@averikitsch
Copy link
Contributor

IdP requires a GCP project but not a Firebase project. Both Google signin and IdP require OAuth 2.0 credentials to be generated.

  • Do they have significant setup latency? We have a requirement for 10 minutes for a new instance.
  • Do they require manual steps to set up? Each manual step complicates setup and any tutorials we create.

IdP has a manual step of enabling IdP. Both Google signin and IdP have dependencies on setting up the credentials, authorized origins and redirects in the manually in console. I have not explored if there are terraform solutions. There shouldn't be any significant differences in setup latency.

  • How do we support local development? Do we need to do something extra to run the app without auth enabled? (E.g., login buttons are no-op and link to pantheon to complete setup)

The OAuth flow won't work without a valid OAuth client Id (and authorized URLs), but won't prevent starting the app. Depending how the logic is to lock down pages may prevent the app from not working without auth.

  • If there is a manual step, does it require redeploying our service?

For both options, you will have to redeploy after adding the client ID to the HTML for login. This could be automated in a script to add a replacement string.

@grayside
Copy link
Collaborator

This issue is part of #46

@ace-n
Copy link
Contributor Author

ace-n commented Jul 22, 2021

Update: I managed to get Identity Platform working with Google Sign-in on the frontend. It generates a token, as well as the email address of the currently signed-in user.

I'm currently working on integrating those changes into the frontend; I'll send a PR once everything's running smoothly.

As part of #120, we'll need to figure out how to forward that token (or something else, like a session token) to the API itself. (@engelke FYI)

@engelke
Copy link
Contributor

engelke commented Jul 22, 2021

I can think of three options.

1 - token is passed as an optional named parameter to client library methods
2 - token is passed when creating an API client object
3 - token is set as a property of the API client object

I expected to generated to automatically do 1. Either I or it (or both) are doing something wrong so far.

@grayside
Copy link
Collaborator

I figure this issue is a stand-in for design work on auth, which has broad implications. As such, here are the items I think we need before proceeding to implementation:

  • Confirm the direction we want to take with auth through wider consensus.
  • Lay out necessary decision records and docs, setting this up is tricky
  • Figure out the demo flows, e.g., do we always want to compel the IdP setup? What happens to app functionality if we don't?
  • How do we keep local development setup easy for contribution?
  • We now have setup scripts and terraform definitions, those need to be kept in sync or work defined for follow-up action. It sounds like any approach with an external dependency will require some back and forth work between Cloud Console and Cloud Run deployment.

This implementation seems like it would make a great friction log, since we definitely want to see this simplified over time, we should use the opportunity to put something together and maybe explore an empathy exercise.

@averikitsch
Copy link
Contributor

A few things from scanning the PR:

  • From my research, the token should never be committed to a cookie.
  • It is interesting to see a login page not just a button.

Let me know if there is anything I can help further with.

@ace-n
Copy link
Contributor Author

ace-n commented Jul 23, 2021

Requiring IdP setup

We have two main options here:

  1. Have an environment variable that disables authentication checks if set
  2. Outright require IdP
  3. Have a "pre-configured" IdP setup with a public demo project

Option 1 would require (re)designing our app itself to assume a "demo user" is logged in when it's running on a local machine. (Certain pages - like user profiles - need some form of user to function properly.)

Option 2 is the easiest to implement, but would of course require additional user setup.

I'm not sure if Option 3 is even feasible (perhaps @averikitsch can weigh in), but it might be worth considering if it is.

Token Storage

Per my understanding, we'll need to identify a user via their browser somehow. I'm OK with not storing the Google-generated token itself in a cookie, but we will likely need to store something there.

(We could add additional layers of indirection - e.g. our own custom session {tokens, management logic} - if we wanted to keep the Google-generated token secret.)

Terraform

It looks like we may be able to obtain the necessary Firebase project config parameters (api_key and auth_domain) from Terraform itself.

Hopefully this will be sufficient to auto-configure new deployments?

Login button

I previously did a button-only implementation via the Google Sign-In API.

It wouldn't be too hard to do here, but I was aiming to put as much logic as possible on the server (i.e. in Python/Jinja) to minimize the frontend's complexity.

Documentation

@grayside I'm guessing you'd like me to document this decision in a (more) {formal, comprehensive} manner outside of GitHub?

If so, let me know and I'll draft something.

@ace-n
Copy link
Contributor Author

ace-n commented Jul 27, 2021

Recording decisions from today's meeting


Requiring IdP setup
Leaning towards Option 2 (require IdP), coupled with only requiring user auth-n for pages where it's necessary (such as a specific user's donation history page)

Token Storage
We're leaning against using cookies directly, but IdP should handle session management for us. If so, we can use IdP directly instead.

Terraform
More research needed; @ace-n to file a follow-up issue.

Anything that cannot be automated (by Terraform) should be documented!

Login button
I didn't see any strong feelings here - we're more constrained by our server-side choices than our client-side ones, so we'll take whatever is easiest for IdP (which, currently, is a specific /login route).

(@averikitsch you had expressed some reservations above; please confirm this conclusion sounds OK to you. 👍 )

Documentation
It seems like GitHub issues (including creating new issues where necessary) are enough for now, so we'll stick to those unless @grayside says otherwise.

@grayside
Copy link
Collaborator

For documentation we are currently missing:

  • In the docs/ directory, a setup README that covers the steps needed to deploy a new instance with use of setup.sh and other automation

Once that is in place, I would expect amending that to be a requirement of any PR that would break those instructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: website Related to the application frontend. needs decision record This issue or change is significant and we should create a decision record. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
4 participants