Centralise all `auth/autz` code
We seem to be spreading authentication code into more and more places, adding a specific methods for different formats.
security is easy to get wrong and forget about some aspects of the system. Especially if we continue reimplementing a well known concepts, and we continue overwrite some aspects of it to inject some behaviours into the process.
We should prefer to always aim on having all authn/authz in a single place to ensure that all aspects of authz are covered, like:
- if PAT is blocked or expired, it is adhered and access is disallowed
- if User is blocked, it is adhered and access is disallowed
- if Build is done, it is adhered and access is disallowed
- and every other constraint that we could have
This means that we should effectively disallow usage of .find_by_token outside of AuthFinders.
How, we could ensure that we allow some methods in a given context? Grape exposes amazing feature
route_setting that allows us to configure some aspects of a route:
route_setting :authentication, job_token_allowed: true
With that approach, we could model multiple auth authentication/authorization models, and ensure that they are in a central place, and enabled only to some.
This is meant to prevent us from doing changes like this:
def find_personal_access_token_from_http_basic_auth
return unless headers
token = decode_token
return unless token
PersonalAccessToken.find_by_token(token)
end
def find_job_from_http_basic_auth
return unless headers
token = decode_token
return unless token
::Ci::Build.find_by_token(token)
end
...
private
def decode_token
encoded_credentials = headers['Authorization'].to_s.split('Basic ', 2).second
Base64.decode64(encoded_credentials || '').split(':', 2).second
end
This model is also slowly being copy-pasted in to the other places as well: !26963 (diffs).
The above would be better if it would be implemented in AuthFinders and be enabled only with:
route_setting :authentication, basic_auth_personal_access_token: true
route_setting :authentication, basic_auth_job_access_token: true