Fix GitLab Service Account enrollment#6479
Conversation
Generated with assistance from Claude Sonnet 4.6 with the following prompts (and then some manual editing): > Create instructions for using the GitLab integration in a file named `gitlab.md`. Recommend creating a project linked service account and then using a PAT from the service account. Flag that this token may need to be rotated using `provider enroll` with the existing provider name when it expires. > Add documentation that explains that you can use either OAuth or a PAT to authorize GitLab, and compares the two approaches using a table > The description to generate a service account PAT is not correct. It should be based on https://docs.gitlab.com/user/profile/service_accounts/#view-and-manage-personal-access-tokens-for-a-service-account, and suggest using the "Service Accounts" page rather than signing in as a different user. > Rewrite step 3 of "Creating a service account and PAT" to better describe the role that should be assigned to the service account. Provide both a recommended maximum-access role and a minimum-bar role, and explain the tradeoffs.
| @@ -46,26 +47,20 @@ func (g *providerClassManager) NewOAuthConfig(_ db.ProviderClass, cli bool) (*oa | |||
| func (*providerClassManager) ValidateCredentials( | |||
There was a problem hiding this comment.
The StoreProviderToken RPC in handlers_oauth.go calls ValidateCredentials with the "AccessToken" RPC field (string type) as an argument. I didn't refactor the naming in the caller, but this is really just "a bearer token that works for auth" and is not necessarily an OAuth access token.
There was a problem hiding this comment.
Makes sense — the naming is a bit misleading but the type switch makes the actual behavior clear. Could be worth a follow-up to rename the field or add a comment in handlers_oauth.go to clarify it accepts any bearer token, not just OAuth tokens
| func tokenNeedsRefresh(token oauth2.Token) bool { | ||
| return !token.Valid() || token.Expiry.UTC().Add(tokenExpirationThreshold).Before(time.Now().UTC()) | ||
| bufferedExpiration := time.Now().UTC().Add(-1 * tokenExpirationThreshold) | ||
| return !strings.HasPrefix(token.AccessToken, "glpat-") && // is a PAT, not an OAuth token |
There was a problem hiding this comment.
Again, we store an (encrypted) JSON object that has the same form as oauth2.Token, but we only set the AccessToken field if it comes from StoreAccessToken. I could also key off the presence of a refresh token or expiry==0 if that feels cleaner than the string-prefix approach.
There was a problem hiding this comment.
The glpat- prefix approach works and is explicit. Keying off expiry==0 or absent refresh token might be more robust long-term if GitLab ever changes their token format, but that seems unlikely. I'd leave it as-is and add a comment explaining why we check the prefix — something like // GitLab PATs always start with "glpat-"; OAuth tokens do not.
|
coverage: 60.475% (+0.03%) from 60.449% — evankanderson:gitlab-service-account into mindersec:main |
|
Reviewed the changes. The type switch in One minor question: Otherwise LGTM. |
3 participants
Summary
#6478 fixed the API validation for GitLab tokens, but the underlying code still expected tokens to be of the "OAuth" format, even when a PAT was explicitly provided. Relax this constraint (which probably fixes GitHub as well, but the app flow for GitHub generally automates the service account creation and token rotation, so we still prefer that for GitHub).
Additionally, simplify ListAllRepositories (used to populate the list in
repo register) for GitLab, using themin_access_levelfilter. This should avoid recommending repos that the token doesn't have sufficient access to.Includes user documentation on setting up GitLab service account PATs.
Testing
Manual testing alongside API double-checks with the
glab apitool. I was able to create a service account ("Custcodian-minder"), assign it a role in the "custcodian" group, and then use it to list and enroll a repository.