screwdriver-cd / screwdriver

An open source build platform designed for continuous delivery.
http://screwdriver.cd
Other
1.01k stars 170 forks source link

Invalid admin token #386

Open FenrirUnbound opened 7 years ago

FenrirUnbound commented 7 years ago

If the pipeline admin's token becomes invalid, then no builds will work for that pipeline.

For a few of our pipelines, the admin's token was invalidated (for reasons beyond our knowing). This caused a variety of issues to come up. More importantly, it was an issue that was difficult to assess.

Observations:

EDIT: Reason for token invalidation

stjohnjohnson commented 7 years ago

One of the ideas we were bouncing around a while back was to disable the pipeline until someone can come in and "take ownership".

Tokens can become invalid if the user revokes the oauth application and some other "unknown" reasons.

nkatzman commented 7 years ago

So I'm seeing this problem a lot with scm-bitbucket

According to the oauth documentation, access tokens are only valid for one hour, and when this happens you get a 401

They say that there is an api route for getting a new access token, but it requires a "refresh token", which was given on the "token grant response" (I think that's the Oauth login response).

This is an annoying bug, because after the 1 hr the webhooks/starting a build won't work unless a relogin is done. I'm down to start addressing it. So where do you think I should start?

My first thought was to add a function to scm-bitbucket for refreshing a token when the response code is 401, but it seem like it's a bigger issue that might need to be addressed in /v4/login, as well as scm-base and maybe scm-github

I'm not sure what the github policy is on oauth access tokens, but this is definitely an issue for bitbucket

d2lam commented 7 years ago

@nkatzman Ugh I think I vaguely remember that problem. For github, token never expires. It sucks bitbucket does this! Can you layout your approach & what you think we need to change? We can discuss some more.

FenrirUnbound commented 7 years ago

A lot of other OAuth-based services do this, since that's how it's defined in the RFC. Youtube APIs and Spotify are some ones that I've recently interacted with that also do this. I believe the flow is to use the access_token until it becomes invalid, then use the refresh_token to renew it and try again.

Ideally, we would need to keep both tokens, which means there would be some refactoring on how we store the secrets. Refactoring scm-base would make a lot of sense in that case.

EDIT: more information

nkatzman commented 7 years ago

After some digging and thinking, it might make sense for scm-base to have functions to check the validity of, and to return a refreshed token

Since the majority of scm functions are passed in a token, it wouldn't make much sense for scm functions to be responsible for refreshing a token

So I'm thinking maybe the right spot to validate/refresh a token is in the UserModel when a token is retrieved. This way, it'll create an invariant where a UserModel always returns a valid token.

One change would then be to the UserModel.unsealToken functionality, where in addition to unsealing the token, it will validate/refresh/save

A concern I have would then be around how often a token is retrieved from the UserModel, and if making these calls to github/bitbucket will add too much latency