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

OAuth / API Token Support #779

Closed
markphelps opened this issue Mar 30, 2022 · 26 comments
Closed

OAuth / API Token Support #779

markphelps opened this issue Mar 30, 2022 · 26 comments
Assignees

Comments

@markphelps
Copy link
Collaborator

This issue is to track/help define adding OAuth (and likely API Token) support in both Flipt backend and Flipt UI. This will allow users to login to Flipt via existing third party services as well as provide basic auth for API requests.

My initial thoughts for this feature are:

  1. Implement OAuth for GitHub/Gitlab only at first. As these seem to me to be the most likely third party services that users of Flipt would also depend on and already have accounts for. This also limits the amount of testing required to validate this feature by limiting the scope at first.
  2. Design the feature in a way that additional providers could be added easily in the future (ex: Google, Azure, etc).
  3. Make OAuth opt-in, as Flipt has not had auth up until this point, this would make the feature backwards compatible.
  4. Since it seems that neither GitHub nor Gitlab support client_credential OAuth Grant type, we'd need a way for API requests (REST/GRPC) to also support auth without a user being present. We could add the ability for users to create API tokens for the machine to machine usecase (your app <-> Flipt). We could also implement scopes for these API tokens such as (management, evaluation) and TTL so that users could limit what these tokens could do.

Further design docs/diagrams/potential DB schema will be added below.

@markphelps
Copy link
Collaborator Author

markphelps commented Mar 30, 2022

Im thinking of this from 'outside in' like how this would work from a user's perspective before diving into the the technical implementation.

Im currently basing the design of this feature off of how grafana has implemented authentication

Here's some docs from grafana:

Config

Here's what their default config looks like for auth, we would use YAML instead, maybe adding TOML support for config would also make sense as an additional feature?

For Flipt I'm thinking the config could look like this:

auth:
  anonymous:
    enabled: true # default since Flipt doesnt have auth now
  github:
    enabled: false # default
    allow_sign_up: true
    client_id: some_id
    client_secret: some_secret
    scopes: user:email,read:org
    auth_url: https://github.com/login/oauth/authorize
    token_url: https://github.com/login/oauth/access_token
    api_url: https://api.github.com/user
    allowed_domains: # csv of allowed domains of user emails on GH to enforce user is part of org
  gitlab:
    enabled: false # default
    allow_sign_up: true
    client_id: some_id
    client_secret: some_secret
    scopes: api
    auth_url:  https://gitlab.com/oauth/authorize
    token_url: https://gitlab.com/oauth/token
    api_url: https://gitlab.com/api/v4
    allowed_domains: # csv of allowed domains of user emails on GL to enforce user is part of org

If anonymous was false then one of the other auth providers would need to be enabled

@tsickert
Copy link
Contributor

Yeah, love that format!

From the user perspective, some sort of support for environment variables for the secret (at least) would be necessary. The scheme you use now where that's prepended with FLIPT_ would work perfectly. So, you'd end up with FLIPT_AUTH_GITHUB_CLIENT_SECRET.

In fact, I wonder if that convention could be used for the entire configuration. So you could provide the auth setup entirely from the environment. If nothing else, that'd be really nice for a docker image example with auth enabled.

As a more advanced feature, you could use some templating in the YAML and then pull the value when the config is parsed.

@markphelps
Copy link
Collaborator Author

yeah, currently every config key/value in YAML can also be set via an ENV var with the FLIPT_ prefix like you mentioned. I plan to keep that going forward as well. Maybe YAML for the non-secret values + ENV var for the secret (client_secret) values is ok for now? Can always add some kind of templating or secret manager integration down the road

@markphelps
Copy link
Collaborator Author

markphelps commented Mar 31, 2022

Alright so I'm starting to think through the backend/frontend flow

Frontend/Backend flow

  1. Login Page if either github or gitlab auth is enabled
  2. User clicks either button
  3. Request goes to backend /login/:provider
  4. Backend does the oauth dance with provider and gets access_token
  5. Backend gets basic user info from provider API using access_token (email, name, user_id)
  6. Backend creates signed JWT that includes access_token, expiry and user info from provider
  7. Backend sets secure cookie with JWT for the frontend to use
  8. Frontend includes JWT in each further call to the backend via Bearer in the request header
  9. Backend verifies the JWT in each request from the frontend to ensure user is logged in

Database

Note: I'm now wondering if we even need any db change at all since the db isnt really part of the oauth flow and theres no real reason to store the user info right now. At worst if we add a new feature that depends on having a persisted user, we'd just need to force users to re-login, which would then populate the user table.

Leaving this note for posterity though:

Actually the more I think about it we will need to persist users to support access tokens created by users, unless we just want to make access tokens global that can created by anyone with no owner

For DB I actually think we'll only need a users table, and thats mainly for future-proofing, for example if we want flags to be linked to users sometime in the future.

users table

column type comment
id VARCHAR(255) UNIQUE NOT NULL primary key, chose varchar so we could use uuid's to be consistent with existing
email VARCHAR(255) UNIQUE NOT NULL
name VARCHAR(255) NOT NULL do we need name? maybe for future audit log support?
enabled boolean prob wont be used right now, but it might be nice for admins to be able to disable users in the future?
created_at TIMESTAMP NOT NULL
updated_at TIMESTAMP NOT NULL

Pretty simple to start

@tsickert
Copy link
Contributor

tsickert commented Apr 1, 2022

For the flow, is this going to be using a standard authorization code grant type? If it is, I think it's going to be more like:

  1. Login Page if either github or gitlab auth is enabled
  2. User clicks either button
  3. User goes to oauth_url page and authenticates
  4. Request is sent to redirect url as configured in app (which would be the portal)
  5. Portal sends request to /login/:provider?code=<provided_code_from_redirect> to backend
  6. Backend exchanges code for access_token
  7. Backend gets basic user info from provider API using access_token (email, name, user_id)
  8. Backend creates signed JWT that includes access_token, expiry and user info from provider
  9. Backend sets secure cookie with JWT for the frontend to use
  10. Frontend includes JWT in each further call to the backend via Bearer in the request header
  11. Backend verifies the JWT in each request from the frontend to ensure user is logged in

If we don't want to do the cookie step, the JWT could be stored in application storage.

The DB schema looks good in principal, but my few thoughts:

I'm on the fence about name. When I've previously implemented oauth, I was very careful to only ask users for information I absolutely needed to make the app work and for name, I feel like YAGNI.

Another thing I'd think about is account linking. The underlying user is a Flipt user that has a Flipt ID. So, when somoene logs in with GitHub and they have the email handle@gmail.com and then go to log in with another provider that also has handle@gmail.com, you may want to say "hey, that user has a globally unique identifier outside of our system that will be associated with 2 accounts, we should link these so I don't make another Flipt user". That can be a join table with a schema like:

users_provider table

column type comment
user_id VARCHAR(255) UNIQUE NOT NULL The ID of the user from the users table
provider VARCHAR (255) NOT NULL The name of the provider (ex: github, gitlab)
provider_id VARCHAR(255) NOT NULL The ID of the user that the provider uses to identify them

Then that table can be used whenever a login happens and you want to associate a provider ID with a Flipt user.

@markphelps
Copy link
Collaborator Author

@tsickert yeah 💯 . I mixed up/left out a few steps. You're right the request would go to the provider then redirect back to us on the backend.

I was wondering how we would handle the case you mentioned about a user being created in the system and then authing with multiple providers. I think a join table makes a lot of sense like you described. I also agree that we can leave out name for now as its probably not needed.

tl;dr: yes to everything you said

@tsickert
Copy link
Contributor

tsickert commented Apr 4, 2022

So, what's your vision for how the authz flows work?

I was thinking about it, and there's sort of a chicken and egg problem. Let's assume that the portal is deployed but is not inside a VPC or it's behind a cloudflare WAF that's misconfigured so it's accessible. How would the administrator be able to manage which accounts should actually have those *:write scopes?

In fact, we'd need to have an administrator account, right? Or at least a way to set which account would be an administrator.

My first thought is that the oauth configuration could have an admin section where emails could be set. That way, when an account logs in with that email, it could be assigned the admin role and then that user could manage the others.

After that, accounts that sign up would probably need to be validated by an admin, or maybe we could provide some wildcarded field to automatically assign people with *@company.com emails write access.

Thoughts?

@markphelps
Copy link
Collaborator Author

markphelps commented Apr 4, 2022

I was thinking that the allowed_domains in the config would be required to be set, so that once Flipt is deployed with this setting, only users whos emails are in the allowed_domains would be allowed access/actually create a user record.

Ex: allowed_domains: foo.com

Only users whose GitHub or Gitlab emails end in foo.com would be able to signin.

Now this has the potential issue that all Oauth providers would need to verify a user's email before actually returning it as a valid email when getting the user info via their APIs.

I'm pretty sure GitHub does this , but we'd need to double check, and also check that GitLab does the same

@markphelps
Copy link
Collaborator Author

Also Id like to add a 'Securing Flipt' guide to the docs when this is rolled out, detailing some best practices, ie: flipt config shouldnt be accessible/writable by non-trusted users, etc

@tsickert
Copy link
Contributor

tsickert commented Apr 4, 2022

I think the Securing Flipt guide idea is really good.

Yeah, the email thing does feel fragile now that you mention it. It doesn't seem reliable given that some accounts are by phone number rather than email.

While the allowed_domains in the config would definintely work for some cases, I do have concerns for fledgling companies or OSS projects, where it may not be practical to rely on a domain name if that domain ends up being gmail.com.

We might need to make more complex rulesets eventually--something like:

allowed_user_filters:
  - type: email
    patterns:
      - someuser@gmail.com
      - '*@foo.bar'
  - type: phone
    patterns:
      - 123-456-7891
  - type: id
    patterns:
      - 123456789

@markphelps
Copy link
Collaborator Author

Yeah I can see that, however Im not trying to solve every potential use case at the moment without feedback from users. So I'd rather start with the same approach that grafana uses (allowed_domains), maybe structure it similar to the above allowed_user_filters to allow for extension later?

so really just support the single basic email filter at first

@tsickert
Copy link
Contributor

tsickert commented Apr 4, 2022

Yeah, you're right, I'm probably over engineering it for the first pass 😅

Putting my customer hat on for a sec: I will say that I use my personal gmail account for my GitHub, so allowed_domains feels a bit coarse grained.

@markphelps
Copy link
Collaborator Author

markphelps commented Apr 4, 2022

Thats a good point. What about instead of email, we use allowed_organizations? where its a CSV list of org names that the member must be a member of?

Actually, re-looking at the grafana GitHub Oauth docs it looks like they support both team_ids and organizations. I like not having them at the top level though, like how you defined allowed_user_filters with a list of filters as members

@tsickert
Copy link
Contributor

tsickert commented Apr 5, 2022

Oh, I totally missed those before. Those would be perfect!

I also agree that having that object hierarchy is probably preferrable to everything at the top level.

@GeorgeMac
Copy link
Contributor

GeorgeMac commented Sep 8, 2022

👋 Just throwing in two cents here. Went through something similar recently (JWT-based auth for session logic).

OWASP advises the HttpOnly attribute on cookies.

https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#httponly-attribute

This protects consumers with client-side vulnerabilities from accessing the contents of the cookie via the DOM.
However, this means that flipt's own JS won't be able to pull the JWT out of the cookie to be put in the Authorization header.

This should not be a problem if you just drop that step. The JWT is in the Cookie header now already.
The browser will take care of ensuring it is applied to any requests for the same domain.
Flipt will just have to look in the Cookie header for the JWT, not the Authorization header.

Another thought, JWT is very unpopular for session logic currently.
It is super convenient for the kinds of flow described here. I implemented something very similar recently.
Security team was… not so happy with the result.

The arguments against are usually around the (in)ability to manually invalidate a particular session.

The first result when you Google "JWT for session management" is:
https://redis.com/blog/json-web-tokens-jwt-are-dangerous-for-user-sessions/

The result often ends up being that you need to some backend storage, which stores the IDs for the token you want to be invalidated. Which means you have to do a lookup on each session in a DB anyway. To find out if the session has been invalidated. You have to store these at-least until they expire naturally, due to the exp claim on the JWT.

Ultimately, you could argue you might as well have some backend session storage, somewhere.
It comes with the added benefit of visibility into what sessions are currently active, as well.

Just something to consider. JWT (for sessions) is still valid, and can ultimately be done securely.

@GeorgeMac
Copy link
Contributor

GeorgeMac commented Sep 8, 2022

Follow-up suggestion would be to do OIDC.

OAuth is historically for delegation of authorization specifically.
Where OIDC is the extension to OAuth for supporting specifically authentication concerns.

Supporting this would open doors to like Okta, Auth0, ORY, Dex and other OIDC providers.

@markphelps
Copy link
Collaborator Author

markphelps commented Sep 8, 2022

I think we'd like to support 2-3 logins via external identity providers out of the box, ranked in order of (my) perceived need:

  1. GitHub
  2. Google
  3. Gitlab

I dont believe GitHub supports OIDC for login directly, although it seems Google does.

But I do agree that supporting OIDC would be ideal, I guess that means we'd still need to support GitHub separately?

@GeorgeMac
Copy link
Contributor

GeorgeMac commented Sep 8, 2022

Yes, Google does. Perhaps doing Google via OIDC would be a beneficial endeavour.

That sounds like a sensible three to start with 🎉

Pointing back to Grafana as a source implementation, here is their "Generic OAuth" impementation.

@stale
Copy link

stale bot commented Dec 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 28, 2022
@markphelps markphelps added keep and removed stale labels Dec 28, 2022
@markphelps markphelps unpinned this issue Jan 2, 2023
@GeorgeMac
Copy link
Contributor

We have support for OIDC and token authentication as of v1.18.0. Enjoy.

@endigma
Copy link

endigma commented Aug 14, 2023

So is GitHub auth not included in this OIDC capability? I don't believe GitHub provides OIDC support and that seems to have been forgotten here

@GeorgeMac
Copy link
Contributor

Hey @endigma . You are right that Github doesn't support OIDC.
We put the breaks on adding Github as no one has yet to actually request it.
However, it was recently discussed internally again. Would you be interested in Github OAuth support?

@endigma
Copy link

endigma commented Aug 14, 2023

We don't have any other suitable auth provider in our org, so very much so yes.

@markphelps
Copy link
Collaborator Author

@endigma we will prioritize this! I think it might unlock some other nice integrations with GitHub down the road, but Oauth support for GitHub is a first step/requirement!

@markphelps
Copy link
Collaborator Author

#2004 for tracking

@markphelps
Copy link
Collaborator Author

Hey @endigma , we just released this in https://github.com/flipt-io/flipt/releases/tag/v1.26.0, working on updating the docs now! let us know how it works for you 🎉

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

No branches or pull requests

4 participants