Skip to content
Snippets Groups Projects
Commit 6a68043c authored by Jessie Young's avatar Jessie Young :heart_exclamation:
Browse files

Validate oauth_access_tokens#expires_in not null

* The background migration ran successfully on GPRD: !107701 (comment 1241450867)
* This migration prevent new invalid oauth access token records: !109047
* Validating the not null constraint is the final step https://docs.gitlab.com/ee/development/database/not_null_constraints.html#validate-the-not-null-constraint-next-release
* Minor: also removes monkeypatching of Doorkeeper which we added before
  the migration ran so that the refresh token flow didn't create new
  invalid PATs while the background migration was running. More context
  in that MR: !107306
* #385374

Changelog: changed
parent 32db70fd
No related branches found
No related tags found
2 merge requests!113305Draft: Sync compliance report parameters with URL,!112765Validate oauth_access_tokens#expires_in not null
......@@ -4,6 +4,8 @@ class OauthAccessToken < Doorkeeper::AccessToken
belongs_to :resource_owner, class_name: 'User'
belongs_to :application, class_name: 'Doorkeeper::Application'
validates :expires_in, presence: true
alias_attribute :user, :resource_owner
scope :latest_per_application, -> { select('distinct on(application_id) *').order(application_id: :desc, created_at: :desc) }
......
......@@ -121,17 +121,3 @@
# We might want to disable this in the future, see https://gitlab.com/gitlab-org/gitlab/-/issues/323615
skip_client_authentication_for_password_grant true
end
module Doorkeeper
class AccessToken
# Doorkeeper OAuth Token refresh uses expires_in of refresh token for new token
# https://github.com/doorkeeper-gem/doorkeeper/pull/1366
# This override ensures that tokens with expires_in: nil do not create new
# tokens with expires_in: nil during refresh flow.
# Can be removed after https://gitlab.com/gitlab-org/gitlab/-/issues/386094 is
# closed
def expires_in
super || 2.hours
end
end
end
# frozen_string_literal: true
class ValidateNotNullConstraintOnOauthAccessTokensExpiresIn < Gitlab::Database::Migration[2.1]
disable_ddl_transaction!
def up
validate_not_null_constraint :oauth_access_tokens, :expires_in
end
def down
# no-op
end
end
1d43fc6bfb88caf86d02b83c944c143bc87142a49f3fe1ec4c54e29c960060c5
\ No newline at end of file
......@@ -18791,7 +18791,8 @@ CREATE TABLE oauth_access_tokens (
expires_in integer DEFAULT 7200,
revoked_at timestamp without time zone,
created_at timestamp without time zone NOT NULL,
scopes character varying
scopes character varying,
CONSTRAINT check_70f294ef54 CHECK ((expires_in IS NOT NULL))
);
 
CREATE SEQUENCE oauth_access_tokens_id_seq
......@@ -26174,9 +26175,6 @@ ALTER TABLE ONLY chat_teams
ALTER TABLE vulnerability_scanners
ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID;
 
ALTER TABLE oauth_access_tokens
ADD CONSTRAINT check_70f294ef54 CHECK ((expires_in IS NOT NULL)) NOT VALID;
ALTER TABLE sprints
ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID;
 
......@@ -59,7 +59,7 @@
it 'uses the expires_in value' do
token = OauthAccessToken.new(expires_in: 1.minute)
expect(token.expires_in).to eq 1.minute
expect(token).to be_valid
end
end
......@@ -67,7 +67,7 @@
it 'uses default value' do
token = OauthAccessToken.new(expires_in: nil)
expect(token.expires_in).to eq 2.hours
expect(token).to be_invalid
end
end
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment