Remove support non OAuth tokens that don't expire

changed milestone to %15.0

added breaking change devopsmanage groupauthentication and authorization [DEPRECATED] labels

mentioned in merge request !69514 (merged)

Setting label(s) ~"Category:Authentication and Authorization" sectiondev based on ~"group::access".

added sectiondev + 1 deleted label

@dblessing, please can you add a type label to this issue to help with issue discovery in issue reports.

added auto updated label

added [deprecated] Accepting merge requests label

added typefeature label

added typemaintenance label and removed typefeature label

added typefeature label

@hsutor Is ~"feature::maintenance" accurate for a deprecation/removal? I have no idea. I didn't see any other type label to match better.

@dblessing I'm checking on your label question and will get back to you...

I read this in the deprecations portion of the handbook:

A deprecation is an announcement in the release post notifying the community of a future removal. Deprecations should be included as soon as possible in the documentation or for at least 2 releases prior to the final removal.

If we are going to deprecate this in 15.0, I suppose I should make a task for myself to write this pending deprecation up in the release posts for 14.4 onwards? @ogolowinski can you see if I'm on the right track?

@hsutor Yes, I think we can announce the deprecation alongside the announcement for expiration support.

@hsutor yes! You should create an issue for deprecation, similar to #337384 (closed) for the next milestone and create a release deprecation note for it following the new process. This will automatically add this to the deprecation docs until %15.0 where we will need to make a removal notice.

It is important to add to the release post, what action the user needs to do in order to rectify the situation before it breaks in %15.0.

@ogolowinski Ok, I created an issue for myself for 14.4 so I don't forget about it. I linked it here.

added to epic &638 (closed)

mentioned in issue #341310 (closed)

marked this issue as related to #341310 (closed)

removed typefeature label

mentioned in issue #351182 (closed)

@jarka Here's one more I didn't catch earlier that will be a %15.0 removal. I will add it to the planning issue now.

mentioned in issue gitlab-org/manage/general-discussion#17455

mentioned in merge request gitlab-com/www-gitlab-com!97640 (closed)

mentioned in merge request !84112 (merged)

NOTE: We will also need a database migration to set an expiration for existing tokens. Otherwise, they will never expire unless revoked (even refreshing a previously non-expiring token will result in a new token without expiration).

Can this occur before the support for non-expiring tokens is dropped? We're attempting to migrate prior to the removal date, and we'd prefer to not have to revoke all existing tokens.

@dblessing ^ something to consider when you work on this in %15.0

Unfortunately we're already too close to the %14.10 release to get anything in. This will all ship in %15.0

Setting weight to 3, also considering the migration needed for this issue. @dblessing feel free to change the weight if you disagree.

set weight to 3

mentioned in epic gitlab-com/support&197 (closed)

added workflowready for development label

mentioned in issue gitlab-org/quality/triage-reports#7347 (closed)

assigned to @ifarkas

Thanks for working on this @ifarkas! We've removed the Seeking community contributions label to avoid having multiple people working on the same issue.

removed [deprecated] Accepting merge requests label

mentioned in issue gitterHQ/webapp#2838 (closed)

added workflowin dev label and removed workflowready for development label

mentioned in merge request !86362 (merged)

mentioned in merge request !86379 (merged)

mentioned in issue gitlab-org/manage/general-discussion#17509

added quad-planningcomplete-no-action label

added workflowverification label and removed workflowin dev label

@dblessing, we shipped 2 MRs as part of 15.0 release:

The background migration is completed. I checked the database, the migration set expires_in for all oauth_access_tokens. However, since the migration is completed, approximately 40% of tokens is created with expires_in set to nil.

Do you know what could we missing here?

The token in doorkeeper is created in Doorkeeper::OAuth::Authorization::Token#issue_token! and the default value of access_token_expires_in is set here: https://github.com/doorkeeper-gem/doorkeeper/blob/v5.5.0.rc2/lib/doorkeeper/config.rb#L266.

We thought this change (!86379 (merged)) was scheduled as part of the May 22nd release. Our team has a change that was scheduled to go out later today around using refresh tokens, but all of our access tokens are now expired.

How can we proceed here without requiring all of our customers to reauthenticate?

@ddorosz Does your integration currently store refresh tokens?

@ifarkas Do we have a node that's not updated?

@dblessing We have a change that was scheduled to go out this week to store and use refresh tokens, but currently we don't have the refresh tokens.

Again, we thought the token expiry changes were part of the GitLab 15.0 release on May 22.

Is there any way to roll this back or reset the expires_in to nil so we can grab refresh tokens without having to reauthenticate our many GitLab apps?

@ifarkas Beyond some instances not being updated/restarted, the only other thing I can think of is some sort of race condition during the migration where maybe some existing tokens were refreshed before the expires_in value was migrated, which would have generated a new token also without expires_in, which may not have been included in the migration loop?

Do we see this number holding steady or are we still getting new tokens without expiry? That might give us some indication if we just need another migration pass or we have a bug somewhere.

Do we see this number holding steady or are we still getting new tokens without expiry?

@dblessing, I'll check the percentage again tomorrow to see how the numbers are changing.

Again, we thought the token expiry changes were part of the GitLab 15.0 release on May 22.

@ddorosz, GitLab.com is already on 15.0 (15.0.0-pre c566a6ecfc8 to be precise). There might have been misunderstanding / miscommunication regarding breaking changes, but May 22 is the release date for self-managed packages.

Thanks for that context @ifarkas.

When I log in to GitLab.com, I see the following:

That led me to believe that GitLab.com itself was also upgrading on May 22.

Unfortunately, I don't think there's a way to rollback right now. For many breaking changes like this one, we offer the ability to opt-in and begin using the new functionality ahead of the breaking change to make the migration easier. We're reviewing the way we communicated the breaking changes on .com to ensure we make it more clear in the future.

Thanks @dblessing (I'm @admin_opslevel above). Yeah, I appreciate that if the migration was essentially:

relation = OAuthAccessTokens
relation.where(expires_in: nil).update_all(expires_in: 7_200)

that it'd be difficult / impossible to roll that back without knowing which tokens had their expiry set.

I have to ask... any way to reset expires_in back to nil for tokens associated with a particular OAuth app?

Hey there, we (vercel.com) had an incident following this rollout. It's on us as we were not using refresh_tokens (we now are).

One thing we were wondering: is it possible that, as part of the migration, some previous refresh tokens were revoked?

Thanks!

On Netlify the Git Gateway access token feature seems to be broken as well, needing to manually refresh the access tokens for all clients each two hours. Is there a way to change this setting within Gitlab? Thank you for your help

@vvo No, refresh tokens should remain valid as part of the migration. Unless the access token was specifically revoked subsequently.

@dylandavies Netlify Git Gateway needs to store the refresh token and request a new access token after the access token expires. The expires_in value when initially getting an access token indicates how long it's valid for. The refresh token can be used anytime during or after that period of time to refresh the token.

@dblessing @ifarkas Can we close this issue or should we keep it open?

@hsutor, I am still verifying if the background migration has been successfully completed. I am currently looking into an issue described in #340848 (comment 949500475). I still need to dig a bit deeper, but I might create a follow-up.