Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session

Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session. Pass these tokens to the project import data.

This prevents the need to encrypt these tokens and clear them in case they expire or get revoked.

For example, if you deleted and re-created OAuth2 keys for Bitbucket, you would get an Error 500 with no way to recover:

Started GET "/import/bitbucket/status" for x.x.x.x at 2015-08-07 05:24:10 +0000
Processing by Import::BitbucketController#status as HTML
Completed 500 Internal Server Error in 607ms (ActiveRecord: 2.3ms)
    
NameError (uninitialized constant Import::BitbucketController::Unauthorized):
  app/controllers/import/bitbucket_controller.rb:77:in `rescue in go_to_bitbucket_for_permissions'
  app/controllers/import/bitbucket_controller.rb:74:in `go_to_bitbucket_for_permissions'
  app/controllers/import/bitbucket_controller.rb:86:in `bitbucket_unauthorized'

Closes #1871 (closed)

Added 1 commit:

• 0be79012 - Clear and retry user's Bitbucket access tokens in case OAuth keys were changed

I'm not so sure about this solution, it seems a bit convoluted. I wonder if there is a better way to detect invalid OAuth configuration rather than trying twice and seeing if it still says Unauthorized?

In any case, this is something that applies too all OAuth-based importers. Would it be possible to solve it for all of them at once?

Reassigned to @stanhu

It looks like GitHub and Bitbucket are the only OAuth-based importers at the moment that store credentials in the user tables.

Perhaps this should be revised to do an explicit access_token verification as the first step:

http://stackoverflow.com/questions/12296017/how-to-validate-a-oauth2-0-access-token-for-a-resource-server

There are security implications with storing access_token and access_token_secret in the database:

http://security.stackexchange.com/questions/72475/should-we-store-accesstoken-in-our-database-for-oauth2

It sounds like we either have to make these encrypted fields or nix them from the database entirely. The latter will force users to reauthenticate with the provider each time. That doesn't seem like such a bad thing given these tokens can expire.

Thoughts, @jacobvosmaer and @ayufan?

Title changed from Clear and retry user's Bitbucket access tokens in case OAuth keys were changed to WIP: Clear and retry user's Bitbucket access tokens in case OAuth keys were changed

@stanhu When using OAuth to sign in to GitLab, it's fine to do the authentication each time, but with importing that's not an option, as each individual import worker needs the access token to get access to projects and issues. I think encrypting them in the DB is the best option.

mentioned in issue #2241 (closed)

What @DouweM says makes sense.

Just for my understanding, these tokens that we are storing in the database, I hope they do expire? Even with encrypted DB fields I don't think a GitLab server should contain more and more valuable tokens for other services over time.

What about storing these access tokens in the session store and passing them to the workers? That seems cleaner than trying to manage the expiration/security of tokens in the database.

Added 143 commits:

• 0be79012...f6f58cfb - 142 commits from branch gitlab-org:master
• 539da4c7 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Added 1 commit:

• 113ddecf - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Title changed from WIP: Clear and retry user's Bitbucket access tokens in case OAuth keys were changed to Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session

Added 1 commit:

• 9fd77a06 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Added 1 commit:

• 16214cef - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Added 1 commit:

• 0431f407 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Added 1 commit:

• 29f5795a - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

@DouweM Ok, got this to work without having to store secret OAuth access tokens in the DB. Would appreciate a review here, particularly this:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1112/diffs#820cff2b7b0e5ee7412452d36a4b50767985657d_100_100

Reassigned to @DouweM

Added 1 commit:

• 48491b98 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

@jacobvosmaer Good idea--done.

Added 1 commit:

• 7a0b849e - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Actually, perhaps instead of passing the token along as worker option, perhaps it would be better to use the project's import_data as this PR does:

https://github.com/gitlabhq/gitlabhq/pull/9538/files

@stanhu nice

Added 1 commit:

• e8ea2404 - Add IV and salt

Added 27 commits:

• e8ea2404...87ec6ae3 - 26 commits from branch gitlab-org:master
• e14cb202 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Title changed from Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session to WIP: Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session

Turns out encrypting the worker options to Sidekiq introduces all sorts of annoying issues with encoding to UTF-8 and decoding back to ASCII-8bit. I think we're better off using a temporary attr_encrypted field in the database.

Reassigned to @stanhu

Added 1 commit:

• 0357fe52 - Encode/decode properly to make Redis/Sidekiq play nicely

Added 1 commit:

• 510361ab - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Added 161 commits:

• 510361ab...97cc91d2 - 160 commits from branch gitlab-org:master
• a390cbfc - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Title changed from WIP: Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session to Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session

Added 1 commit:

• a58b69a6 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

I've updated this MR to use import_data instead. I think we'll have to revisit storing the token in the project's import_url and consider putting this into import_data as well.

Reassigned to @DouweM

Added 1 commit:

• ed1d4fa4 - Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab

Status changed to merged

mentioned in commit 24b282ae

I'm this issue, how i can remove manually the bitbucket oauth for my user=? (i think i have to go inside the database, but i have got no idea)

mentioned in merge request !4064 (closed)