gocd / gocd

GoCD - Continuous Delivery server main repository
https://www.gocd.org
Apache License 2.0
7.11k stars 973 forks source link

Epic: Access Token based authentication support for GoCD APIs #5321

Closed vishaldevgire closed 5 years ago

vishaldevgire commented 6 years ago
Issue Type
Summary

Currently GoCD APIs support basic authentication, this issue is about adding support for access token-based authentication to GoCD APIs.

For 19.2.0

Bugs

Can be done post-release

Next release

Won't Fix

arvindsv commented 5 years ago

If you're thinking about the API, I'd consider the POST / PUT taking a request body in which it has something like:

"permissions": "all"

or

"permissions": [
    {
        "/go/api/*": true
    }
]

... something like that. To make it easier to get people used to possible changes around granular auth tokens. However, since we will have versioning, maybe it's ok not to. Thinking out loud here.

arvindsv commented 5 years ago

Probably best to not bother about it now, and let versioning help us out later?

maheshp commented 5 years ago

@rahulpargaonkar to test this feature.

arvindsv commented 5 years ago

@maheshp, @rajiesh and I had a discussion about this and here's what we came up with:

Assumptions:

Just to be clear. Feel free to tell us our assumptions are wrong.

Scenarios to think about:

1. Scenario: Person removed from LDAP
  1. User is on LDAP and an auth token is created.
  2. User is then removed from LDAP (not just change of groups).
  3. User is not disabled in GoCD.
  4. Makes an API call using the token.

How does GoCD know that they're not allowed to access the API? Does authorization call fail?

2. Scenario: Lost roles when using multiple auth configurations
  1. User with username "user1" has access to group1 through LDAP auth configuration.
  2. User with username "user1" has access to group2 through GitHub auth configuration.
  3. User logs in using LDAP (Session 1).
  4. User logs in using GitHub in a different browser (Session 2).
  5. Session 1 now loses access to group1.

This bug is not specific to auth tokens. Happens with normal browser sessions too. This is because the store which has the user to role mapping is keyed by username, and not my (username + plugin ID) combination. This happens when the username is the same across multiple authorization configurations (usually across plugins).

We considered whether (username + plugin ID) is enough. There are possible corner case bugs, but it's good enough. Better than what we have now.

With auth tokens, this problem can become worse. Especially with auth tokens with more granular permissions.

@maheshp's estimation is that this change is very widespread and we're ok with not doing this right now. Could do it when doing more work around granular permissions.

3. Scenario: API token sessions should be different from that of browser requests.

If we reuse a session (based on username), then any authentication token previously stored in the session will get overwritten, causing problems for reauth (you'd lose the GitHub reauth token, for instance).

So, API token requests should have their own session. But, it can be thrown away after the request. It probably doesn't even need to be alive for 5 minutes, like other API sessions are today.

4. Scenario: Multiple auth tokens for a user and revoking one
  1. Two auth tokens against same plugin for same user.
  2. Revoke one.
  3. The other auth token should continue to work, and should have the same roles as earlier.

Might have to keep track of when roles were removed from the store, when revoke happens, so that a reauth can happen for the token which is not revoked.

Questions:

  1. Does super admin have more permissions, to be able to force revoke tokens, or to force a policy of token expiry?
  2. Do auth tokens really need names? How about generated ID + a description, instead? The ID can be used in the URL. If not, the name cannot contain spaces and will have other artificial restrictions.
  3. Is expiry really needed? Maybe later. But, it feels like it's not needed right now. We couldn't think of a usecase where a user would want to limit their own key, especially since it's not granular yet.
  4. It is good practice to never store passwords/tokens in plain text. So, we should find an appropriate, cryptographically secure hash with the right performance profile to use, before storing it in the DB. If you accept that, then the "Get token" call doesn't make sense. Only the "Get token details" one will be possible.

Other thoughts:

We tried to think of usecases for this, and saw if there was anything missing. We thought of:

  1. Auditing (possibly through structured logs using logback - this will make it flexible enough to send to a different system if needed).
  2. "Last used" - All auth token mechanisms seem to keep track of when a token was last used. Seems like useful information to quickly figure out misuse? Need to consider performance of storing this.
  3. Throttling / limiting requests per second for API calls. We felt that it was more of an API gateway concern, rather than an application level concern at this time. So, maybe don't worry about it for now.

/cc @maheshp @rajiesh - Please feel free to add anything else I missed from our conversation.

arvindsv commented 5 years ago

Some of what is mentioned in the comment above is based on https://github.com/gocd/gocd/issues/5354.

GaneshSPatil commented 5 years ago

@gocd/committers -- Here is the gist of all the things we've discussed:

arvindsv commented 5 years ago

Currently, we dont support deletion of auth tokens for auditing purposes.

That means, "You can only revoke, but not delete auth tokens". Right? The reason is "auditing" (to be able to show that a token existed). I wonder how any auditing can use that, but I think it's fine.

varshavaradarajan commented 5 years ago

Will the token be generated only to use with the APIs? Or can it be used in /go/auth/login as one of the authentication means?

arvindsv commented 5 years ago

@varshavaradarajan Only for APIs. At least that's what is mentioned here: https://github.com/gocd/gocd/issues/5321#issuecomment-437297399

adityasood commented 5 years ago

Pending

  1. [x] Documentation for filters in admin access API.(https://github.com/gocd/api.go.cd/pull/295)
  2. [x] Admin SPA user documentation (https://github.com/gocd/docs.go.cd/pull/248)

@kritika-singh3