8000 Lint, fmt, and fix some tests · coder/coder@b7e55ef · GitHub
[go: up one dir, main page]
Skip to content

Commit b7e55ef

Browse files
code-ashercode-asher
committed
Lint, fmt, and fix some tests
1 parent fe2c91c commit b7e55ef

File tree

13 files changed

+239
-237
lines changed

13 files changed

+239
-237
lines changed

‎coderd/apikey/apikey_test.go

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@ import (
1010
"github.com/stretchr/testify/assert"
1111
"github.com/stretchr/testify/require"
1212

13-
"github.com/coder/coder/v2/cli/clibase"
1413
"github.com/coder/coder/v2/coderd/apikey"
1514
"github.com/coder/coder/v2/coderd/database"
1615
"github.com/coder/coder/v2/coderd/database/dbtime"
17-
"github.com/coder/coder/v2/codersdk"
1816
)
1917

2018
func TestGenerate(t *testing.T) {
@@ -30,69 +28,80 @@ func TestGenerate(t *testing.T) {
3028
{
3129
name: "OK",
3230
params: apikey.CreateParams{
33-
UserID: uuid.New(),
34-
LoginType: database.LoginTypeOIDC,
35-
DeploymentValues: &codersdk.DeploymentValues{},
36-
ExpiresAt: time.Now().Add(time.Hour),
37-
LifetimeSeconds: int64(time.Hour.Seconds()),
38-
TokenName: "hello",
39-
RemoteAddr: "1.2.3.4",
40-
Scope: database.APIKeyScopeApplicationConnect,
31+
UserID: uuid.New(),
32+
LoginType: database.LoginTypeOIDC,
33+
DefaultLifetime: time.Duration(0),
34+
ExpiresAt: time.Now().Add(time.Hour),
35+
LifetimeSeconds: int64(time.Hour.Seconds()),
36+
TokenName: "hello",
37+
RemoteAddr: "1.2.3.4",
38+
Scope: database.APIKeyScopeApplicationConnect,
4139
},
4240
},
4341
{
4442
name: "InvalidScope",
4543
params: apikey.CreateParams{
46-
UserID: uuid.New(),
47-
LoginType: database.LoginTypeOIDC,
48-
DeploymentValues: &codersdk.DeploymentValues{},
49-
ExpiresAt: time.Now().Add(time.Hour),
50-
LifetimeSeconds: int64(time.Hour.Seconds()),
51-
TokenName: "hello",
52-
RemoteAddr: "1.2.3.4",
53-
Scope: database.APIKeyScope("test"),
44+
UserID: uuid.New(),
45+
LoginType: database.LoginTypeOIDC,
46+
DefaultLifetime: time.Duration(0),
47+
ExpiresAt: time.Now().Add(time.Hour),
48+
LifetimeSeconds: int64(time.Hour.Seconds()),
49+
TokenName: "hello",
50+
RemoteAddr: "1.2.3.4",
51+
Scope: database.APIKeyScope("test"),
5452
},
5553
fail: true,
5654
},
5755
{
5856
name: "DeploymentSessionDuration",
5957
params: apikey.CreateParams{
60-
UserID: uuid.New(),
61-
LoginType: database.LoginTypeOIDC,
62-
DeploymentValues: &codersdk.DeploymentValues{
63-
SessionDuration: clibase.Duration(time.Hour),
64-
},
58+
UserID: uuid.New(),
59+
LoginType: database.LoginTypeOIDC,
60+
DefaultLifetime: time.Hour,
6561
LifetimeSeconds: 0,
6662
ExpiresAt: time.Time{},
6763
TokenName: "hello",
6864
RemoteAddr: "1.2.3.4",
6965
Scope: database.APIKeyScopeApplicationConnect,
7066
},
7167
},
68+
{
69+
name: "LifetimeSeconds",
70+
params: apikey.CreateParams{
71+
UserID: uuid.New(),
72+
LoginType: database.LoginTypeOIDC,
73+
DefaultLifetime: time.Duration(0),
74+
LifetimeSeconds: int64(time.Hour.Seconds()),
75+
ExpiresAt: time.Time{},
76+
TokenName: "hello",
77+
RemoteAddr: "1.2.3.4",
78+
Scope: database.APIKeyScopeApplicationConnect,
79+
},
80+
},
7281
{
7382
name: "DefaultIP",
7483
params: apikey.CreateParams{
75-
UserID: uuid.New(),
76-
LoginType: database.LoginTypeOIDC,
77-
DeploymentValues: &codersdk.DeploymentValues{},
78-
ExpiresAt: time.Now().Add(time.Hour),
79-
LifetimeSeconds: int64(time.Hour.Seconds()),
80-
TokenName: "hello",
81-
RemoteAddr: "",
82-
Scope: database.APIKeyScopeApplicationConnect,
84+
UserID: uuid.New(),
85+
LoginType: database.LoginTypeOIDC,
86+
DefaultLifetime: time.Duration(0),
87+
ExpiresAt: time.Now().Add(time.Hour),
88+
LifetimeSeconds: int64(time.Hour.Seconds()),
89+
TokenName: "hello",
90+
RemoteAddr: "",
91+
Scope: database.APIKeyScopeApplicationConnect,
8392
},
8493
},
8594
{
8695
name: "DefaultScope",
8796
params: apikey.CreateParams{
88-
UserID: uuid.New(),
89-
LoginType: database.LoginTypeOIDC,
90-
DeploymentValues: &codersdk.DeploymentValues{},
91-
ExpiresAt: time.Now().Add(time.Hour),
92-
LifetimeSeconds: int64(time.Hour.Seconds()),
93-
TokenName: "hello",
94-
RemoteAddr: "1.2.3.4",
95-
Scope: "",
97+
UserID: uuid.New(),
98+
LoginType: database.LoginTypeOIDC,
99+
DefaultLifetime: time.Duration(0),
100+
ExpiresAt: time.Now().Add(time.Hour),
101+
LifetimeSeconds: int64(time.Hour.Seconds()),
102+
TokenName: "hello",
103+
RemoteAddr: "1.2.3.4",
104+
Scope: "",
96105
},
97106
},
98107
}
@@ -131,15 +140,15 @@ func TestGenerate(t *testing.T) {
131140
// Should not be a delta greater than 5 seconds.
132141
assert.InDelta(t, time.Until(tc.params.ExpiresAt).Seconds(), key.LifetimeSeconds, 5)
133142
} else {
134-
assert.Equal(t, int64(tc.params.DeploymentValues.SessionDuration.Value().Seconds()), key.LifetimeSeconds)
143+
assert.Equal(t, int64(tc.params.DefaultLifetime.Seconds()), key.LifetimeSeconds)
135144
}
136145

137146
if !tc.params.ExpiresAt.IsZero() {
138147
assert.Equal(t, tc.params.ExpiresAt.UTC(), key.ExpiresAt)
139148
} else if tc.params.LifetimeSeconds > 0 {
140-
assert.WithinDuration(t, dbtime.Now().Add(time.Duration(tc.params.LifetimeSeconds)), key.ExpiresAt, time.Second*5)
149+
assert.WithinDuration(t, dbtime.Now().Add(time.Duration(tc.params.LifetimeSeconds)*time.Second), key.ExpiresAt, time.Second*5)
141150
} else {
142-
assert.WithinDuration(t, dbtime.Now().Add(tc.params.DeploymentValues.SessionDuration.Value()), key.ExpiresAt, time.Second*5)
151+
assert.WithinDuration(t, dbtime.Now().Add(tc.params.DefaultLifetime), key.ExpiresAt, time.Second*5)
143152
}
144153

145154
if tc.params.RemoteAddr != "" {

‎coderd/coderdtest/oidctest/helper.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"database/sql"
66
"encoding/json"
7-
"fmt"
87
"net/http"
98
"net/url"
109
"testing"
@@ -162,7 +161,7 @@ func OAuth2GetCode(authURL string, state string, doRequest func(req *http.Reques
162161

163162
newState := toURL.Query().Get("state")
164163
if newState != state {
165-
return "", fmt.Errorf("expected state %q, got %q", state, newState)
164+
return "", xerrors.Errorf("expected state %q, got %q", state, newState)
166165
}
167166
return code, nil
168167
}

‎coderd/coderdtest/oidctest/idp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ func (f *FakeIDP) ExternalLogin(t testing.TB, client *codersdk.Client, opts ...f
424424
// unit tests, it's easier to skip this step sometimes. It does make an actual
425425
// request to the IDP, so it should be equivalent to doing this "manually" with
426426
// actual requests.
427-
func (f *FakeIDP) CreateAuthCode(t testing.TB, state string, opts ...func(r *http.Request)) string {
427+
func (f *FakeIDP) CreateAuthCode(t testing.TB, state string) string {
428428
// We need to store some claims, because this is also an OIDC provider, and
429429
// it expects some claims to be present.
430430
f.stateToIDTokenClaims.Store(state, jwt.MapClaims{})

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func OAuth2ProviderApp(accessURL *url.URL, dbApp database.OAuth2ProviderApp) cod
234234
Icon: dbApp.Icon,
235235
Endpoints: codersdk.OAuth2ProviderAppEndpoints{
236236
Authorization: accessURL.ResolveReference(&url.URL{
237-
Path: fmt.Sprintf("/login/oauth2/authorize"),
237+
Path: "/login/oauth2/authorize",
238238
}).String(),
239239
Token: accessURL.ResolveReference(&url.URL{
240240
Path: "/login/oauth2/tokens",

‎coderd/database/modelmethods.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,11 +281,11 @@ func (c OAuth2ProviderAppCode) RBACObject() rbac.Object {
281281
return rbac.ResourceOAuth2ProviderAppCodeToken.WithOwner(c.UserID.String())
282282
}
283283

284-
func (s OAuth2ProviderAppSecret) RBACObject() rbac.Object {
284+
func (OAuth2ProviderAppSecret) RBACObject() rbac.Object {
285285
return rbac.ResourceOAuth2ProviderAppSecret
286286
}
287287

288-
func (s OAuth2ProviderApp) RBACObject() rbac.Object {
288+
func (OAuth2ProviderApp) RBACObject() rbac.Object {
289289
return rbac.ResourceOAuth2ProviderApp
290290
}
291291

‎enterprise/coderd/coderd.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
167167
r.Get("/authorize", api.postOAuth2ProviderAppAuthorize())
168168
r.Post("/tokens", api.postOAuth2ProviderAppToken())
169169
})
170-
171170
})
172171
})
173172

‎enterprise/coderd/identityprovider/authorize.go

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ import (
1818
"github.com/coder/coder/v2/cryptorand"
1919
)
2020

21-
// TODO: Comment this up
21+
/**
22+
* Authorize displays an HTML for authorizing an application when the user has
23+
* first been redirected to this path and generates a code and redirects to the
24+
* app's callback URL after the user clicks "allow" on that page.
25+
*/
2226
func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
2327
handler := func(rw http.ResponseWriter, r *http.Request) {
2428
// TODO: audit? and other endpoints?
25-
// TODO: @Emyrk this has to serve the html payload for clicking "allow".
26-
// We need a method to determine if someone hit this endpoint clicking "Allow",
27-
// or they got here for the first time.
2829
ctx := r.Context()
2930
apiKey, ok := httpmw.APIKeyOptional(r)
3031
if !ok {
@@ -34,17 +35,13 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
3435
return
3536
}
3637

37-
// If this request is coming from a different host, we need to show the
38-
// user an html page where they can click allow.
39-
// TODO: Is there a smarter way to do this?
40-
// TODO: Skip this step if the user has already clicked allow before, and we
41-
// can just reuse the token.
42-
4338
app := httpmw.OAuth2ProviderApp(r)
4439

4540
// TODO: @emyrk this should always work, maybe make callbackURL a *url.URL?
4641
callbackURL, _ := url.Parse(app.CallbackURL)
4742

43+
// TODO: Should validate these on the HTML page as well and show errors
44+
// there rather than wait until this endpoint to show them.
4845
p := httpapi.NewQueryParamParser()
4946
vals := r.URL.Query()
5047
p.Required("state", "response_type")
@@ -75,7 +72,7 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
7572
}
7673

7774
// TODO: @emyrk handle scope?
78-
var _ = scope
75+
_ = scope
7976

8077
if err := validateRedirectURL(app.CallbackURL, redirectURL.String()); err != nil {
8178
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
@@ -91,7 +88,6 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
9188
})
9289
return
9390
}
94-
// TODO: We are ignoring scopes for now.
9591
hashed := sha256.Sum256([]byte(rawSecret))
9692
_, err = db.InsertOAuth2ProviderAppCode(ctx, database.InsertOAuth2ProviderAppCodeParams{
9793
ID: uuid.New(),
@@ -110,8 +106,6 @@ func Authorize(db database.Store, accessURL *url.URL) http.HandlerFunc {
110106
return
111107
}
112108

113-
// TODO: @emyrk we need to possibly serve an html page from this endpoint.
114-
// this currently skips the user in the loop part where they click allow.
115109
newQuery := redirectURL.Query()
116110
newQuery.Add("code", rawSecret)
117111
newQuery.Add("state", state)
@@ -135,9 +129,9 @@ func validateRedirectURL(baseURL string, redirectURL string) error {
135129
if err != nil {
136130
return err
137131
}
138-
// It can be a sub-domain or a sub-directory.
139-
if (redirect.Host != base.Host && !strings.HasSuffix(redirect.Host, "."+base.Host)) ||
140-
!strings.HasPrefix(redirect.Path, base.Path) {
132+
// It can be a sub-directory but not a sub-domain, as we have apps on
133+
// sub-domains so it seems too dangerous.
134+
if redirect.Host != base.Host || !strings.HasPrefix(redirect.Path, base.Path) {
141135
return xerrors.New("invalid redirect URL")
142136
}
143137
return nil

‎enterprise/coderd/identityprovider/middleware.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ func authorizeMW(accessURL *url.URL) func(next http.Handler) http.Handler {
3939

4040
app := httpmw.OAuth2ProviderApp(r)
4141
// If the request comes from outside, then we show the html allow page.
42+
// TODO: Skip this step if the user has already clicked allow before, and
43+
// we can just reuse the token.
4244
if originU.Hostname() != accessURL.Hostname() && refererU.Path != "/login/oauth2/authorize" {
4345
if r.URL.Query().Get("redirected") != "" {
4446
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{

‎enterprise/coderd/identityprovider/tokens.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
5555
}
5656

5757
// TODO: Not sure what to do with this yet.
58-
var _ = redirectURL
58+
_ = redirectURL
5959

6060
var token oauth2.Token
6161
switch codersdk.OAuth2ProviderGrantType(grantType) {

‎enterprise/coderd/oauth2_test.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,16 @@ func TestOAuth2ProviderApps(t *testing.T) {
202202

203203
// Should be able to add apps.
204204
expected := generateApps(ctx, t, client)
205+
expectedOrder := []codersdk.OAuth2ProviderApp{
206+
expected.Default, expected.NoPort, expected.Subdomain,
207+
expected.Extra[0], expected.Extra[1],
208+
}
205209

206210
// Should get all the apps now.
207211
apps, err = another.OAuth2ProviderApps(ctx, codersdk.OAuth2ProviderAppFilter{})
208212
require.NoError(t, err)
209213
require.Len(t, apps, 5)
210-
require.Equal(t, expected, apps)
214+
require.Equal(t, expectedOrder, apps)
211215

212216
// Should be able to keep the same name when updating.
213217
req := codersdk.PutOAuth2ProviderAppRequest{
@@ -252,13 +256,7 @@ func TestOAuth2ProviderApps(t *testing.T) {
252256
require.NoError(t, err)
253257
require.Len(t, newApps, 4)
254258

255-
filtered := make([]codersdk.OAuth2ProviderApp, 0, len(newApps))
256-
for _, app := range newApps {
257-
if app.ID != expected.Default.ID {
258-
filtered = append(filtered, app)
259-
}
260-
}
261-
require.Equal(t, filtered, newApps)
259+
require.Equal(t, expectedOrder[1:], newApps)
262260
})
263261

264262
t.Run("ByUser", func(t *testing.T) {
@@ -632,14 +630,13 @@ func TestOAuth2ProviderTokenExchange(t *testing.T) {
632630
code = *test.defaultCode
633631
} else {
634632
code, err = authorizationFlow(userClient, valid)
635-
if test.authError == "" {
636-
require.NoError(t, err)
637-
} else {
633+
if test.authError != "" {
638634
require.Error(t, err)
639635
require.ErrorContains(t, err, test.authError)
640636
// If this errors the token exchange will fail. So end here.
641637
return
642638
}
639+
require.NoError(t, err)
643640
}
644641

645642
// Mutate the valid config for the exchange.

‎scripts/renderstatics/main.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"log"
55
"net/http"
6+
"time"
67

78
"github.com/go-chi/chi/v5"
89

@@ -26,12 +27,17 @@ func main() {
2627
Warnings: []string{"The world will end in 7 days.", "Please panic."},
2728
})
2829
default:
29-
site.RenderOAuthAllowPage(rw, r, http.StatusOK, site.RenderOAuthAllowData{
30-
AppName: "Netflix Backstage",
30+
site.RenderOAuthAllowPage(rw, r, site.RenderOAuthAllowData{
31+
AppName: "Backstage",
3132
Icon: "",
3233
RedirectURI: "https://google.com",
3334
})
3435
}
3536
}))
36-
log.Fatal(http.ListenAndServe(":7775", r))
37+
server := &http.Server{
38+
Addr: ":7775",
39+
ReadHeaderTimeout: 20 * time.Second,
40+
Handler: r,
41+
}
42+
log.Fatal(server.ListenAndServe())
3743
}

‎site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { type FC, useState } from "react";
22
import { useMutation, useQuery, useQueryClient } from "react-query";
33
import { getErrorMessage } from "api/errors";
44
import { getApps, revokeApp } from "api/queries/oauth2";
5-
import type * as TypesGen from "api/typesGenerated";
65
import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog";
76
import { displayError, displaySuccess } from "components/GlobalSnackbar/utils";
87
import { useMe } from "hooks";

0 commit comments

Comments
 (0)
0