8000 refactoring unit tests and how we pass args to api key gen · coder/coder@6ef27fc · GitHub
[go: up one dir, main page]
Skip to content

Commit 6ef27fc

Browse files
EmyrkEmyrk
committed
refactoring unit tests and how we pass args to api key gen
1 parent 8e0f121 commit 6ef27fc

File tree

13 files changed

+681
-493
lines changed

13 files changed

+681
-493
lines changed

‎coderd/apikey.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
8282
}
8383

8484
cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{
85-
UserID: user.ID,
86-
LoginType: database.LoginTypeToken,
87-
DeploymentValues: api.DeploymentValues,
88-
ExpiresAt: dbtime.Now().Add(lifeTime),
89-
Scope: scope,
90-
LifetimeSeconds: int64(lifeTime.Seconds()),
91-
TokenName: tokenName,
85+
UserID: user.ID,
86+
LoginType: database.LoginTypeToken,
87+
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
88+
ExpiresAt: dbtime.Now().Add(lifeTime),
89+
Scope: scope,
90+
LifetimeSeconds: int64(lifeTime.Seconds()),
91+
TokenName: tokenName,
9292
})
9393
if err != nil {
9494
if database.IsUniqueViolation(err, database.UniqueIndexAPIKeyName) {
@@ -127,10 +127,10 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
127127

128128
lifeTime := time.Hour * 24 * 7
129129
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
130-
UserID: user.ID,
131-
DeploymentValues: api.DeploymentValues,
132-
LoginType: database.LoginTypePassword,
133-
RemoteAddr: r.RemoteAddr,
130+
UserID: user.ID,
131+
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
132+
LoginType: database.LoginTypePassword,
133+
RemoteAddr: r.RemoteAddr,
134134
// All api generated keys will last 1 week. Browser login tokens have
135135
// a shorter life.
136136
ExpiresAt: dbtime.Now().Add(lifeTime),

‎coderd/apikey/apikey.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ import (
1212

1313
"github.com/coder/coder/v2/coderd/database"
1414
"github.com/coder/coder/v2/coderd/database/dbtime"
15-
"github.com/coder/coder/v2/codersdk"
1615
"github.com/coder/coder/v2/cryptorand"
1716
)
1817

1918
type CreateParams struct {
20-
UserID uuid.UUID
21-
LoginType database.LoginType
22-
DeploymentValues *codersdk.DeploymentValues
19+
UserID uuid.UUID
20+
LoginType database.LoginType
21+
// DefaultLifetime is configured in DeploymentValues.
22+
// It is used if LifetimeSeconds is not set.
23+
DefaultLifetime time.Duration
2324

2425
// Optional.
2526
ExpiresAt time.Time
@@ -46,8 +47,8 @@ func Generate(params CreateParams) (database.InsertAPIKeyParams, string, error)
4647
if params.LifetimeSeconds != 0 {
4748
params.ExpiresAt = dbtime.Now().Add(time.Duration(params.LifetimeSeconds) * time.Second)
4849
} else {
49-
params.ExpiresAt = dbtime.Now().Add(params.DeploymentValues.SessionDuration.Value())
50-
params.LifetimeSeconds = int64(params.DeploymentValues.SessionDuration.Value().Seconds())
50+
params.ExpiresAt = dbtime.Now().Add(params.DefaultLifetime)
51+
params.LifetimeSeconds = int64(params.DefaultLifetime.Seconds())
5152
}
5253
}
5354
if params.LifetimeSeconds == 0 {

‎coderd/coderdtest/oidctest/helper.go

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ import (
44
"context"
55
"database/sql"
66
"encoding/json"
7+
"fmt"
78
"net/http"
8-
"net/http/httputil"
99
"net/url"
1010
"testing"
1111
"time"
1212

1313
"github.com/golang-jwt/jwt/v4"
1414
"github.com/stretchr/testify/require"
15+
"golang.org/x/xerrors"
1516

1617
"github.com/coder/coder/v2/coderd/database"
1718
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -124,36 +125,43 @@ func (h *LoginHelper) ForceRefresh(t *testing.T, db database.Store, user *coders
124125
// actual requests.
125126
//
126127
// TODO: Is state param optional? Can we grab it from the authURL?
127-
func OAuth2GetCode(t testing.TB, authURL string, state string, doRequest func(req *http.Request) (*http.Response, error)) string {
128-
t.Helper()
129-
128+
func OAuth2GetCode(authURL string, state string, doRequest func(req *http.Request) (*http.Response, error)) (string, error) {
130129
// We need to store some claims, because this is also an OIDC provider, and
131130
// it expects some claims to be present.
132131
// TODO: POST or GET method?
133132
r, err := http.NewRequestWithContext(context.Background(), http.MethodGet, authURL, nil)
134-
require.NoError(t, err, "failed to create auth request")
133+
if err != nil {
134+
return "", xerrors.Errorf("failed to create auth request: %w", err)
135+
}
135136

136137
expCode := http.StatusTemporaryRedirect
137138
resp, err := doRequest(r)
138-
if err == nil && resp.StatusCode != expCode {
139-
data, _ := httputil.DumpResponse(resp, true)
140-
t.Log("Auth URL", authURL)
141-
t.Log("HTTP Response Dump", string(data))
139+
if err != nil {
140+
return "", xerrors.Errorf("request: %w", err)
141+
}
142+
143+
if resp.StatusCode != expCode {
144+
return "", codersdk.ReadBodyAsError(resp)
142145
}
143-
require.NoError(t, err, "request failed")
144146

145-
require.Equal(t, expCode, resp.StatusCode, "expected redirect")
146147
to := resp.Header.Get("Location")
147-
require.NotEmpty(t, to, "expected redirect location")
148+
if to == "" {
149+
return "", xerrors.Errorf("expected redirect location")
150+
}
148151

149152
toURL, err := url.Parse(to)
150-
require.NoError(t, err, "failed to parse redirect location")
153+
if err != nil {
154+
return "", xerrors.Errorf("failed to parse redirect location: %w", err)
155+
}
151156

152157
code := toURL.Query().Get("code")
153-
require.NotEmpty(t, code, "expected code in redirect location")
158+
if code == "" {
159+
return "", xerrors.Errorf("expected code in redirect location")
160+
}
154161

155162
newState := toURL.Query().Get("state")
156-
require.Equal(t, state, newState, "expected state to match")
157-
158-
return code
163+
if newState != state {
164+
return "", fmt.Errorf("expected state %q, got %q", state, newState)
165+
}
166+
return code, nil
159167
}

‎coderd/coderdtest/oidctest/idp.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,12 +429,14 @@ func (f *FakeIDP) CreateAuthCode(t testing.TB, state string, opts ...func(r *htt
429429
// it expects some claims to be present.
430430
f.stateToIDTokenClaims.Store(state, jwt.MapClaims{})
431431

432-
return OAuth2GetCode(t, f.cfg.AuthCodeURL(state), state, func(req *http.Request) (*http.Response, error) {
432+
code, err := OAuth2GetCode(f.cfg.AuthCodeURL(state), state, func(req *http.Request) (*http.Response, error) {
433433
rw := httptest.NewRecorder()
434434
f.handler.ServeHTTP(rw, req)
435435
resp := rw.Result()
436436
return resp, nil
437437
})
438+
require.NoError(t, err, "failed to get auth code")
439+
return code
438440
}
439441

440442
// OIDCCallback will emulate the IDP redirecting back to the Coder callback.

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,11 +1682,11 @@ func workspaceSessionTokenName(workspace database.Workspace) string {
16821682

16831683
func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) {
16841684
newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{
1685-
UserID: user.ID,
1686-
LoginType: user.LoginType,
1687-
DeploymentValues: s.DeploymentValues,
1688-
TokenName: workspaceSessionTokenName(workspace),
1689-
LifetimeSeconds: int64(s.DeploymentValues.MaxTokenLifetime.Value().Seconds()),
1685+
UserID: user.ID,
1686+
LoginType: user.LoginType,
1687+
DefaultLifetime: s.DeploymentValues.SessionDuration.Value(),
1688+
TokenName: workspaceSessionTokenName(workspace),
1689+
LifetimeSeconds: int64(s.DeploymentValues.MaxTokenLifetime.Value().Seconds()),
16901690
})
16911691
if err != nil {
16921692
return "", xerrors.Errorf("generate API key: %w", err)

‎coderd/userauth.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,10 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
247247

248248
//nolint:gocritic // Creating the API key as the user instead of as system.
249249
cookie, key, err := api.createAPIKey(dbauthz.As(ctx, userSubj), apikey.CreateParams{
250-
UserID: user.ID,
251-
LoginType: database.LoginTypePassword,
252-
RemoteAddr: r.RemoteAddr,
253-
DeploymentValues: api.DeploymentValues,
250+
UserID: user.ID,
251+
LoginType: database.LoginTypePassword,
252+
RemoteAddr: r.RemoteAddr,
253+
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
254254
})
255255
if err != nil {
256256
logger.Error(ctx, "unable to create API key", slog.Error(err))
@@ -1544,10 +1544,10 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
15441544
} else {
15451545
//nolint:gocritic
15461546
cookie, newKey, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), apikey.CreateParams{
1547-
UserID: user.ID,
1548-
LoginType: params.LoginType,
1549-
DeploymentValues: api.DeploymentValues,
1550-
RemoteAddr: r.RemoteAddr,
1547+
UserID: user.ID,
1548+
LoginType: params.LoginType,
1549+
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
1550+
RemoteAddr: r.RemoteAddr,
15511551
})
15521552
if err != nil {
15531553
return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err)

‎coderd/workspaceapps.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
112112
lifetimeSeconds = int64(api.DeploymentValues.SessionDuration.Value().Seconds())
113113
}
114114
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
115-
UserID: apiKey.UserID,
116-
LoginType: database.LoginTypePassword,
117-
DeploymentValues: api.DeploymentValues,
118-
ExpiresAt: exp,
119-
LifetimeSeconds: lifetimeSeconds,
120-
Scope: database.APIKeyScopeApplicationConnect,
115+
UserID: apiKey.UserID,
116+
LoginType: database.LoginTypePassword,
117+
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
118+
ExpiresAt: exp,
119+
LifetimeSeconds: lifetimeSeconds,
120+
Scope: database.APIKeyScopeApplicationConnect,
121121
})
122122
if err != nil {
123123
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

‎enterprise/coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
165165
r.Route("/login", func(r chi.Router) {
166166
r.Route("/oauth2", func(r chi.Router) {
167167
r.Get("/authorize", api.postOAuth2ProviderAppAuthorize())
168-
r.Post("/tokens", api.postOAuth2ProviderAppToken)
168+
r.Post("/tokens", api.postOAuth2ProviderAppToken())
169169
})
170170

171171
})

‎enterprise/coderd/groups_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ func TestGroups(t *testing.T) {
799799

800800
groups, err := userAdminClient.GroupsByOrganization(ctx, user.OrganizationID)
801801
require.NoError(t, err)
802-
// 'Everyone' group + 2 custom groups.
802+
// 'Everyone' group + 2 exchangeMutate groups.
803803
require.Len(t, groups, 3)
804804
require.Contains(t, groups, group1)
805805
require.Contains(t, groups, group2)

0 commit comments

Comments
 (0)
0