mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: expose external auth token expiry in agent API and CLI (#26883)
Previously, \`ExternalAuthResponse\` contained no expiry information, so workspace agents and git credential helpers had no way to know when a cached token would stop being valid. Every git operation had to call back to coderd via \`GIT_ASKPASS\` to get a fresh token, adding 1-2 seconds of latency. This PR surfaces \`OAuthExpiry\` from the database as \`ExpiresAt\` in \`ExternalAuthResponse\`, allowing agents to cache tokens with correct eviction timing (compatible with \`git-credential-cache --timeout\` and \`password_expiry_utc\` introduced in git 2.34). \`ExpiresAt\` is normalized to UTC before JSON encoding to avoid sub-minute precision loss that occurs when the PostgreSQL driver applies historical Local Mean Time (LMT) timezone offsets to year-1 AD timestamps. The \`coder external-auth access-token\` CLI command gains \`--output json\` to print the full response including \`ExpiresAt\`, enabling scripts to consume the expiry without parsing heuristics. Closes https://github.com/coder/coder/issues/26036 ## Manual Test <details> <summary>Setup</summary> 1. Create a GitHub OAuth app at https://github.com/settings/developers with: - Homepage URL: `http://127.0.0.1:3000` - Authorization callback URL: `http://127.0.0.1:3000/external-auth/github/callback` 2. Start the dev server with the GitHub provider configured: ```sh CODER_EXTERNAL_AUTH_0_ID=github CODER_EXTERNAL_AUTH_0_TYPE=github CODER_EXTERNAL_AUTH_0_CLIENT_ID=<client-id> CODER_EXTERNAL_AUTH_0_CLIENT_SECRET=<client-secret> ./scripts/develop.sh ``` 3. Log in at `http://127.0.0.1:3000` (use `127.0.0.1`, not `localhost`, so the OAuth state cookie domain matches the callback URL). 4. Go to Account > External Authentication and click **Connect** next to GitHub. Complete the OAuth flow. 5. Create a workspace and SSH into it: ```sh coder create test-workspace coder ssh test-workspace ``` </details> <details> <summary>Flow 1: Token is valid — JSON output includes <code>expires_at</code></summary> Inside the workspace, run: ```sh coder external-auth access-token github --output json echo "Exit code: $?" ``` Expected output (GitHub tokens have no expiry, so \`expires_at\` is the zero value): ```json { "access_token": "<redacted>", "token_extra": null, "url": "", "type": "github", "expires_at": "0001-01-01T00:00:00Z", "username": "<redacted>", "password": "" } ``` ``` Exit code: 0 ``` </details> <details> <summary>Flow 2: Token missing — JSON output includes auth URL, exit code 1</summary> Disconnect GitHub in the Coder UI (Account > External Authentication > Disconnect), then inside the workspace run: ```sh coder external-auth access-token github --output json echo "Exit code: $?" ``` Expected output: ```json { "access_token": "", "token_extra": null, "url": "http://127.0.0.1:3000/external-auth/github", "type": "", "expires_at": "0001-01-01T00:00:00Z", "username": "", "password": "" } ``` ``` Exit code: 1 ``` </details>
This commit is contained in:
Generated
+4
@@ -14631,6 +14631,10 @@ const docTemplate = `{
|
||||
"access_token": {
|
||||
"type": "string"
|
||||
},
|
||||
"expires_at": {
|
||||
"description": "ExpiresAt is the time the token expires, normalized to UTC (for\nexample, \"2024-06-01T15:04:05Z\"). Zero value means no expiry.",
|
||||
"type": "string"
|
||||
},
|
||||
"password": {
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
Generated
+4
@@ -12983,6 +12983,10 @@
|
||||
"access_token": {
|
||||
"type": "string"
|
||||
},
|
||||
"expires_at": {
|
||||
"description": "ExpiresAt is the time the token expires, normalized to UTC (for\nexample, \"2024-06-01T15:04:05Z\"). Zero value means no expiry.",
|
||||
"type": "string"
|
||||
},
|
||||
"password": {
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
@@ -2135,7 +2135,7 @@ func (api *API) workspaceAgentsExternalAuth(rw http.ResponseWriter, r *http.Requ
|
||||
})
|
||||
return
|
||||
}
|
||||
resp, err := createExternalAuthResponse(externalAuthConfig.Type, refreshedLink.OAuthAccessToken, refreshedLink.OAuthExtra)
|
||||
resp, err := createExternalAuthResponse(externalAuthConfig.Type, refreshedLink.OAuthAccessToken, refreshedLink.OAuthExtra, refreshedLink.OAuthExpiry)
|
||||
if err != nil {
|
||||
handleRetrying(http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to create external auth response.",
|
||||
@@ -2208,7 +2208,7 @@ func (api *API) workspaceAgentsExternalAuthListen(ctx context.Context, rw http.R
|
||||
if !valid {
|
||||
continue
|
||||
}
|
||||
resp, err := createExternalAuthResponse(externalAuthConfig.Type, externalAuthLink.OAuthAccessToken, externalAuthLink.OAuthExtra)
|
||||
resp, err := createExternalAuthResponse(externalAuthConfig.Type, externalAuthLink.OAuthAccessToken, externalAuthLink.OAuthExtra, externalAuthLink.OAuthExpiry)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to create external auth response.",
|
||||
@@ -2375,7 +2375,7 @@ func fillCoderDesktopTelemetry(r *http.Request, event *telemetry.UserTailnetConn
|
||||
// createExternalAuthResponse creates an ExternalAuthResponse based on the
|
||||
// provider type. This is to support legacy `/workspaceagents/me/gitauth`
|
||||
// which uses `Username` and `Password`.
|
||||
func createExternalAuthResponse(typ, token string, extra pqtype.NullRawMessage) (agentsdk.ExternalAuthResponse, error) {
|
||||
func createExternalAuthResponse(typ, token string, extra pqtype.NullRawMessage, expiry time.Time) (agentsdk.ExternalAuthResponse, error) {
|
||||
var resp agentsdk.ExternalAuthResponse
|
||||
switch typ {
|
||||
case string(codersdk.EnhancedExternalAuthProviderGitLab):
|
||||
@@ -2398,6 +2398,10 @@ func createExternalAuthResponse(typ, token string, extra pqtype.NullRawMessage)
|
||||
}
|
||||
resp.AccessToken = token
|
||||
resp.Type = typ
|
||||
// Normalize to UTC so JSON encoding always uses the "Z" suffix and
|
||||
// preserves the full timestamp without losing sub-minute precision from
|
||||
// historical timezone offsets (e.g. LMT).
|
||||
resp.ExpiresAt = expiry.UTC()
|
||||
|
||||
var err error
|
||||
if extra.Valid {
|
||||
|
||||
@@ -13,9 +13,11 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/google/uuid"
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/xerrors"
|
||||
@@ -32,6 +34,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/rbac/policy"
|
||||
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/codersdk/agentsdk"
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
|
||||
"github.com/coder/coder/v2/codersdk/wsjson"
|
||||
@@ -979,3 +982,112 @@ func TestWatchAgentContainers(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestCreateExternalAuthResponse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Use a fixed future time.
|
||||
expiry := dbtime.Now().Add(8 * time.Hour).UTC()
|
||||
|
||||
assertExpiry := func(t *testing.T, resp agentsdk.ExternalAuthResponse, want time.Time) {
|
||||
t.Helper()
|
||||
require.Equal(t, want.UTC(), resp.ExpiresAt.UTC(),
|
||||
"ExpiresAt should match the expiry passed to createExternalAuthResponse")
|
||||
}
|
||||
|
||||
t.Run("WithExpiry", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resp, err := createExternalAuthResponse("github", "tok", pqtype.NullRawMessage{}, expiry)
|
||||
require.NoError(t, err)
|
||||
assertExpiry(t, resp, expiry)
|
||||
require.Equal(t, "tok", resp.AccessToken)
|
||||
})
|
||||
|
||||
t.Run("ZeroExpiry", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A zero expiry means the token never expires. ExpiresAt should stay zero.
|
||||
resp, err := createExternalAuthResponse("github", "tok", pqtype.NullRawMessage{}, time.Time{})
|
||||
require.NoError(t, err)
|
||||
require.True(t, resp.ExpiresAt.IsZero(), "ExpiresAt should be zero when no expiry is set")
|
||||
})
|
||||
|
||||
// Each provider type maps the token into a different Username/Password pair.
|
||||
// All of them must also carry ExpiresAt through unchanged.
|
||||
providerTests := []struct {
|
||||
name string
|
||||
typ string
|
||||
token string
|
||||
wantUsername string
|
||||
wantPassword string
|
||||
}{
|
||||
{
|
||||
name: "GitHub",
|
||||
typ: codersdk.EnhancedExternalAuthProviderGitHub.String(),
|
||||
token: "ghtoken",
|
||||
wantUsername: "ghtoken",
|
||||
wantPassword: "",
|
||||
},
|
||||
{
|
||||
name: "GitLab",
|
||||
typ: codersdk.EnhancedExternalAuthProviderGitLab.String(),
|
||||
token: "gltoken",
|
||||
wantUsername: "oauth2",
|
||||
wantPassword: "gltoken",
|
||||
},
|
||||
{
|
||||
name: "BitbucketCloud",
|
||||
typ: codersdk.EnhancedExternalAuthProviderBitBucketCloud.String(),
|
||||
token: "bbtoken",
|
||||
wantUsername: "x-token-auth",
|
||||
wantPassword: "bbtoken",
|
||||
},
|
||||
{
|
||||
name: "BitbucketServer",
|
||||
typ: codersdk.EnhancedExternalAuthProviderBitBucketServer.String(),
|
||||
token: "bbtoken",
|
||||
wantUsername: "x-token-auth",
|
||||
wantPassword: "bbtoken",
|
||||
},
|
||||
}
|
||||
for _, tt := range providerTests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resp, err := createExternalAuthResponse(tt.typ, tt.token, pqtype.NullRawMessage{}, expiry)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, tt.wantUsername, resp.Username)
|
||||
require.Equal(t, tt.wantPassword, resp.Password)
|
||||
require.Equal(t, tt.token, resp.AccessToken)
|
||||
assertExpiry(t, resp, expiry)
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("WithTokenExtra", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
extra := pqtype.NullRawMessage{
|
||||
RawMessage: []byte(`{"user_id":"u_42","scope":"repo"}`),
|
||||
Valid: true,
|
||||
}
|
||||
resp, err := createExternalAuthResponse("slack", "slacktoken", extra, expiry)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "u_42", resp.TokenExtra["user_id"])
|
||||
require.Equal(t, "repo", resp.TokenExtra["scope"])
|
||||
assertExpiry(t, resp, expiry)
|
||||
})
|
||||
|
||||
t.Run("InvalidExtraJSON", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Malformed JSON in the extra field should produce an error but
|
||||
// ExpiresAt should still reflect the expiry that was passed in.
|
||||
extra := pqtype.NullRawMessage{
|
||||
RawMessage: []byte(`not-valid-json`),
|
||||
Valid: true,
|
||||
}
|
||||
_, err := createExternalAuthResponse("github", "tok", extra, expiry)
|
||||
require.Error(t, err, "malformed extra JSON should produce an error")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -25,6 +26,7 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/oauth2"
|
||||
"golang.org/x/xerrors"
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
"tailscale.com/tailcfg"
|
||||
@@ -3698,3 +3700,105 @@ func (p *pubsubReinitSpy) Subscribe(event string, listener pubsub.Listener) (can
|
||||
p.Unlock()
|
||||
return cancel, err
|
||||
}
|
||||
|
||||
// TestWorkspaceAgentsExternalAuthExpiresAt verifies that the expiry stored on
|
||||
// an ExternalAuthLink is returned in ExternalAuthResponse.ExpiresAt via the
|
||||
// full HTTP round-trip, covering both a non-zero and zero expiry.
|
||||
func TestWorkspaceAgentsExternalAuthExpiresAt(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const providerID = "test-provider"
|
||||
|
||||
// seedToken is both the access token value stored in the DB and the one
|
||||
// the fake OAuth2 provider returns. When they match, RefreshToken detects
|
||||
// no change and skips the DB update, preserving the seeded OAuthExpiry.
|
||||
const seedToken = "seed-token"
|
||||
|
||||
// newSetup creates a coderdtest server with a minimal external-auth
|
||||
// provider that has no ValidateURL (all tokens accepted as valid) and
|
||||
// returns seedToken so that RefreshToken does not overwrite the link.
|
||||
newSetup := func(t *testing.T) (agentToken string, agentClient *agentsdk.Client, db database.Store, ownerID uuid.UUID) {
|
||||
t.Helper()
|
||||
|
||||
ownerClient, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
|
||||
ExternalAuthConfigs: []*externalauth.Config{{
|
||||
InstrumentedOAuth2Config: &testutil.OAuth2Config{
|
||||
// Return seedToken so token.AccessToken == originalAccessToken
|
||||
// in RefreshToken, preventing a DB update that would overwrite
|
||||
// the seeded OAuthExpiry.
|
||||
Token: &oauth2.Token{
|
||||
AccessToken: seedToken,
|
||||
RefreshToken: "refresh-token",
|
||||
Expiry: dbtime.Now().Add(24 * time.Hour),
|
||||
},
|
||||
},
|
||||
ID: providerID,
|
||||
Regex: regexp.MustCompile(`.*`),
|
||||
Type: codersdk.EnhancedExternalAuthProviderGitHub.String(),
|
||||
// ValidateURL intentionally omitted: tokens are always valid.
|
||||
}},
|
||||
})
|
||||
first := coderdtest.CreateFirstUser(t, ownerClient)
|
||||
_, user := coderdtest.CreateAnotherUser(t, ownerClient, first.OrganizationID)
|
||||
|
||||
r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
|
||||
OrganizationID: first.OrganizationID,
|
||||
OwnerID: user.ID,
|
||||
}).WithAgent().Do()
|
||||
|
||||
ac := agentsdk.New(ownerClient.URL, agentsdk.WithFixedToken(r.AgentToken))
|
||||
return r.AgentToken, ac, db, user.ID
|
||||
}
|
||||
|
||||
t.Run("NonZeroExpiry", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
_, agentClient, db, userID := newSetup(t)
|
||||
|
||||
// Seed a link with an 8-hour expiry and verify the response carries it.
|
||||
want := dbtime.Now().Add(8 * time.Hour).UTC().Truncate(time.Second)
|
||||
dbgen.ExternalAuthLink(t, db, database.ExternalAuthLink{
|
||||
ProviderID: providerID,
|
||||
UserID: userID,
|
||||
OAuthAccessToken: seedToken,
|
||||
OAuthExpiry: want,
|
||||
})
|
||||
|
||||
resp, err := agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{
|
||||
ID: providerID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, resp.URL, "token should be valid, no redirect URL expected")
|
||||
require.Equal(t, want, resp.ExpiresAt.UTC().Truncate(time.Second),
|
||||
"ExpiresAt should match the expiry stored in the database")
|
||||
})
|
||||
|
||||
t.Run("ZeroExpiry", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
_, agentClient, db, userID := newSetup(t)
|
||||
|
||||
// dbgen.ExternalAuthLink uses takeFirst which skips zero time.Time
|
||||
// values and fills in a 24-hour default. Insert the link directly to
|
||||
// store an explicit zero OAuthExpiry (token never expires).
|
||||
_, err := db.InsertExternalAuthLink(dbauthz.AsSystemRestricted(ctx), database.InsertExternalAuthLinkParams{
|
||||
ProviderID: providerID,
|
||||
UserID: userID,
|
||||
OAuthAccessToken: seedToken,
|
||||
OAuthExpiry: time.Time{},
|
||||
CreatedAt: dbtime.Now(),
|
||||
UpdatedAt: dbtime.Now(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{
|
||||
ID: providerID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, resp.URL)
|
||||
require.True(t, resp.ExpiresAt.IsZero(),
|
||||
"ExpiresAt should be zero when the token has no expiry")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user