Gocd: Using access token causes java.lang.NullPointerException

[GaneshSPatil]

Issue Type

• Bug Report

Summary

In GoCD, personal access tokens belongs to a user. And a user is a part of an authorization configuration, which in turn causes personal access token to belong to an auth config.

Every personal access token has a reference to the belonging auth config using the auth config id.
Every time a personal access token is used, the validity and the authenticity of the personal access token is verified against the belonging auth config id.

When the auth config id(s) are changed, all the belonging access token has a reference to the old auth config ids, which in turn causes a causes java.lang.NullPointerException when such access tokens are used.

Basic environment details

• Go Version: 20.5.0

Steps to Reproduce

The error scenario is explained as follows:

1. Let's say a user Bob belongs to an Authorization Configuration Admins.
2. User Bob creates a personal access token and uses the newly created access token to access GoCD APIs (and everything works).
3. Someone modifies the GoCD configurations by editing the cruise-config.xml (_either on disk or from UI_) and renames the authorization configuration from Admins to SuperAdmins.
4. Now, user Bob again tries to access the GoCD APIs and here, the request fails with 500 error.

Note: This scenario is also applicable for deleted auth configs.

Log snippets

2020-07-10 12:03:46,587 DEBUG [qtp1894369629-25] AccessToken:113 - Generating secret using algorithm: PBKDF2WithHmacSHA256 with spec: DEFAULT_ITERATIONS: 4096, DESIRED_KEY_LENGTH: 256
2020-07-10 12:03:46,593 DEBUG [qtp1894369629-25] AccessToken:131 - [Bearer Token Authentication] Authenticating bearer token for: GoCD User: 'admin'. GoCD API endpoint: '/go/api/current_user', API Client: 'curl/7.64.1', Is Admin Scoped Token: 'true', Current Time: '2020-07-10 12:03:46.593'.
2020-07-10 12:03:46,596 WARN  [qtp1894369629-25] HttpChannel:591 - /go/api/current_user
java.lang.NullPointerException: null
    at com.thoughtworks.go.server.newsecurity.providers.AbstractPluginAuthenticationProvider.authenticateUser(AbstractPluginAuthenticationProvider.java:109)
    at com.thoughtworks.go.server.newsecurity.filters.AccessTokenAuthenticationFilter.filterWhenSecurityEnabled(AccessTokenAuthenticationFilter.java:145)
    at com.thoughtworks.go.server.newsecurity.filters.AccessTokenAuthenticationFilter.doFilterInternal(AccessTokenAuthenticationFilter.java:94)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
    at com.thoughtworks.go.server.newsecurity.filters.AbstractBasicAuthenticationFilter.filterWhenSecurityEnabled(AbstractBasicAuthenticationFilter.java:88)
    at com.thoughtworks.go.server.newsecurity.filters.AbstractBasicAuthenticationFilter.doFilterInternal(AbstractBasicAuthenticationFilter.java:67)

Actual Behavior

Accessing the GoCD APIs using personal access token after renaming the belonging auth config id causes 500 error.

Expected Behavior

Possible Fix 1: GoCD should revoke all the access token when belonging auth config id is changed or deleted.

When an auth config id is modified (by modifying the cruise-config.xml) or deleted, GoCD should revoke all the existing access token, specifying:

1. Revoked Message: Revoked by GoCD: The belonging authorization configuration 'auth-config-id' is deleted.
2. Revoked By: GoCD.

Possible Fix 2: GoCD should show appropriate error when such access tokens are used.

Accessing the GoCD APIs using personal access token after deleting or renaming the belonging auth config id should fail with 400 status code and following message:
Can not find authorization configuration 'auth-config-id' to which the requested personal access token belongs. Authorization Configuration 'auth-config-id' might have been renamed or deleted. Please revoke the existing token and create a new one for the same.

[arvindsv]

In my view, IDs should not be changed. Maybe we should add a Display name field there, and auto-generate the ID and not expect that to be changed. My vote is for "Fix 2".

If we add a display name, we should copy over the current ID as the display name, for existing auth configs.

[kritika-singh3]

No matter we use auto-generate ID or user-defined ID - both of them can be updated via directly editing the cruise-config.xml.
IMO, we can add a description field (like we have on secret_config). The users can update the description as and when they want. (Something name usually means that it should not exceed some characters whereas description can be as long as needed. Hence, my preference)
In case an explicit delete auth_config is called - I would prefer Fix 1. In case of not found - Fix 2.

[kritika-singh3]

Fixed and verified via #8339