The mantra wins again.

We will need to either keep the crypto scope or do the database migration to quietly remove that scope from any applications, access tokens or access grants. If we remove the scope without the database migration, then I think we'll get 500 errors if we attempt to access an application using that scope.

RE: the crypto scope (configured via doorkeeper) - given the commented-out and disabled nature of the feature, it seems unlikely that there are purposefully created records with that scope, doesn't it? Is the concern that since it was a configured scope and there hypothetically could have been requests for that scope which were processed and saved that way, we should be conservative about handling that edge case via migration or other mitigation ... or am I missing something about the creation path and it's actually more likely to have some records?

We thought the same about the read:me scope which had only recently been merged, but in the month between there were many applications already using it. The crypto scope has existed for years, so, I'd expect there'd be at least some applications registered with it. Using the post migration should be enough:

    Doorkeeper::Application.where("scopes LIKE '%crypto%'").in_batches do |applications|
      applications.update_all("scopes = replace(scopes, 'crypto', '')")
    end

    Doorkeeper::AccessToken.where("scopes LIKE '%crypto%'").in_batches do |access_tokens|
      access_tokens.update_all("scopes = replace(scopes, 'crypto', '')")
    end

Should be sufficient. Arguably we should trim whitespace, but the whitespace isn't meaningfully important.

I don't generally suggest doing AccessGrants just because they're highly ephemeral, and the risk of breakage is very very minimal.

This pull request has merge conflicts that must be resolved before it can be merged.

This pull request has resolved merge conflicts and is ready for review.

This pull request has merge conflicts that must be resolved before it can be merged.

This pull request has resolved merge conflicts and is ready for review.

This pull request has merge conflicts that must be resolved before it can be merged.

This pull request has resolved merge conflicts and is ready for review.

@renchap fantastic news! The one thing this still absolutely needs is the migration to drop the OAuth Scopes for crypto, otherwise we'll be having errors & breakages: #31193 (comment)

Sorry, just saw this...

We thought the same about the read:me scope which had only recently been merged, but in the month between there were many applications already using it. The crypto scope has existed for years, so, I'd expect there'd be at least some applications registered with it.

Are those cases the same though? In the case of the read:me scope, it was active and deployed in nightlies and running places ... so while it's reasonable to not expect it to have been super widely used, there's at least a viable path for it to have been used by whatever audience of nightly-running instances there is. Whereas in the crypto scope case, to the best of my understanding is has never actually been enabled anywhere? (or at least, in default unchanged non-forked deployment)

I'll add a migration here or as followup if necessary, but I still don't understand the path where it could have been used.

@mjankowski it existed in the UI when creating an application, was documented and was accepted when creating an application via API; it should likely be silently dropped & future applications should not be able to be created with it.

Just dropping the scope & the locales without rewriting the records will potentially result in applications that when viewed result in a 500

Ah, I see ... I was thinking totally about the inbound permissions checking from the commented-out routes and couldnt see how they'd have created anything. Noted on that pathway/exposure.

I can work on that migration and either add here or as followup if this merges first.

Separately...

• That view could use a responsive once over at narrow width?
• I wonder if the "this application has a scope which the locales dont know about" could be handled more gracefully?