Skip to content

Commit 2f661d3

Browse files
committed
Modify openapi & middleware to allow for optional auth
Typically, you would set the security section to the following to signal that endpoint can optionally take bearerAuth: ``` security: - bearerAuth: [] - {} ``` But the current Go openapi library does not let us know that {} has been set. As a result, this change sets a new security called `noAuth`. Now with this new security parameter, it lets the server know that: 1. If the authorization header is provided, try to authenticate like normal, or 2. If the authorization header is not set and this new noAuth option is a possibility for the route, treat it like an unauthenticated request that should proceed. The GCIP middleware has been modified to fit that logic. This is needed for the get_saved_search endpoint. Other changes: - Have the getSavedSearch endpoint use the new noAuth config - Fix other openapi lint errors.
1 parent 80a88f0 commit 2f661d3

File tree

4 files changed

+114
-37
lines changed

4 files changed

+114
-37
lines changed

backend/cmd/server/main.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ func main() {
136136
}
137137

138138
authMiddleware := httpmiddlewares.NewBearerTokenAuthenticationMiddleware(
139-
auth.NewGCIPAuthenticator(firebaseAuthClient), backend.BearerAuthScopes, httpserver.GenericErrorFn)
139+
auth.NewGCIPAuthenticator(firebaseAuthClient),
140+
backend.BearerAuthScopes,
141+
backend.NoAuthScopes,
142+
httpserver.GenericErrorFn,
143+
)
140144

141145
preRequestMiddlewares := []func(http.Handler) http.Handler{
142146
cors.Handler(

lib/httpmiddlewares/bearertokenauth.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,31 +35,42 @@ type BearerTokenAuthenticator interface {
3535
}
3636

3737
// NewBearerTokenAuthenticationMiddleware returns a middleware that can be used to authenticate requests.
38-
// It detects if a route requires authentication by checking if a field is set in the request context.
39-
// If the field is set, the middleware verifies the Authorization header and sets the authenticated user in the context.
38+
// It detects if a route requires authentication by checking if a field (authCtxKey) is set in the request context.
39+
// If the authCtxKey field is set and the Authorization header is present, the middleware authenticates the user and
40+
// sets the authenticated user in the context. If both authCtxKey and optionalAuthCtxKey fields are set and the
41+
// Authorization header is not present, it allows the request to proceed without authentication.
4042
//
4143
// The errorFn parameter allows the caller to customize the error response returned when authentication fails.
4244
// This makes the middleware more generic and adaptable to different error handling requirements.
4345
//
44-
// It is the responsibility of the caller of this middleware to ensure that the `ctxKey` is set in the request context
45-
// whenever authentication is needed. This can be done using a wrapper middleware that knows about the OpenAPI
46+
// It is the responsibility of the caller of this middleware to ensure that the `authCtxKey` is set in the request
47+
// context whenever authentication is needed. This can be done using a wrapper middleware that knows about the OpenAPI
4648
// generator's security semantics.
4749
//
4850
// See https://github.com/oapi-codegen/oapi-codegen/issues/518 for details on the lack of per-endpoint middleware
4951
// support.
50-
func NewBearerTokenAuthenticationMiddleware(authenticator BearerTokenAuthenticator, ctxKey any,
52+
func NewBearerTokenAuthenticationMiddleware(authenticator BearerTokenAuthenticator,
53+
authCtxKey any, optionalAuthCtxKey any,
5154
errorFn func(context.Context, int, http.ResponseWriter, error)) func(http.Handler) http.Handler {
5255
return func(next http.Handler) http.Handler {
5356
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
54-
value := r.Context().Value(ctxKey)
57+
value := r.Context().Value(authCtxKey)
5558
if value == nil {
5659
// The route does not have any security requirements set for it.
5760
next.ServeHTTP(w, r)
5861

5962
return
6063
}
64+
optionalAuthValue := r.Context().Value(optionalAuthCtxKey)
6165
authHdr := r.Header.Get("Authorization")
6266
// Check for the Authorization header.
67+
if authHdr == "" && optionalAuthValue != nil {
68+
// optionalAuthCtxKey is set and no Authorization header, proceed without authentication.
69+
next.ServeHTTP(w, r)
70+
71+
return
72+
}
73+
6374
if authHdr == "" {
6475
errorFn(r.Context(), http.StatusUnauthorized, w, ErrMissingAuthHeader)
6576

lib/httpmiddlewares/bearertokenauth_test.go

Lines changed: 81 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,14 @@ import (
2727

2828
type authCtxKey struct{}
2929

30+
type optionalAuthCtxKey struct{}
31+
3032
func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
3133
const testID = "id"
3234
tests := []struct {
3335
name string
34-
ctxKey any
36+
authCtxKey any
37+
optionalAuthCtxKey any
3538
authHeader string
3639
mockAuthenticator func(ctx context.Context, token string) (*auth.User, error)
3740
mockErrorFn func(context.Context, int, http.ResponseWriter, error)
@@ -40,9 +43,10 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
4043
expectedUser *auth.User
4144
}{
4245
{
43-
name: "No security requirements",
44-
ctxKey: nil,
45-
authHeader: "",
46+
name: "No security requirements",
47+
authCtxKey: nil,
48+
optionalAuthCtxKey: nil,
49+
authHeader: "",
4650
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
4751
t.Fatal("authenticate should not have been called")
4852

@@ -57,9 +61,10 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
5761
expectedUser: nil,
5862
},
5963
{
60-
name: "Missing Authorization header",
61-
ctxKey: authCtxKey{},
62-
authHeader: "",
64+
name: "Missing Authorization header",
65+
authCtxKey: authCtxKey{},
66+
authHeader: "",
67+
optionalAuthCtxKey: nil,
6368
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
6469
t.Fatal("authenticate should not have been called")
6570

@@ -80,9 +85,10 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
8085
expectedBody: "",
8186
},
8287
{
83-
name: "Invalid Authorization header",
84-
ctxKey: authCtxKey{},
85-
authHeader: "Invalid Auth",
88+
name: "Invalid Authorization header",
89+
authCtxKey: authCtxKey{},
90+
optionalAuthCtxKey: nil,
91+
authHeader: "Invalid Auth",
8692
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
8793
t.Fatal("authenticate should not have been called")
8894

@@ -103,9 +109,10 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
103109
expectedBody: "",
104110
},
105111
{
106-
name: "Authentication failure",
107-
ctxKey: authCtxKey{},
108-
authHeader: "Bearer my-token",
112+
name: "Authentication failure without optional auth context",
113+
authCtxKey: authCtxKey{},
114+
optionalAuthCtxKey: nil,
115+
authHeader: "Bearer my-token",
109116
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
110117
return nil, errors.New("authentication failed")
111118
},
@@ -123,9 +130,49 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
123130
expectedBody: "",
124131
},
125132
{
126-
name: "Successful authentication",
127-
ctxKey: authCtxKey{},
128-
authHeader: "Bearer my-token",
133+
name: "Authentication failure with optional auth context",
134+
authCtxKey: authCtxKey{},
135+
optionalAuthCtxKey: optionalAuthCtxKey{},
136+
authHeader: "Bearer my-token",
137+
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
138+
return nil, errors.New("authentication failed")
139+
},
140+
mockErrorFn: func(_ context.Context, code int, w http.ResponseWriter, err error) {
141+
if code != http.StatusUnauthorized {
142+
t.Errorf("expected status code %d, got %d", http.StatusUnauthorized, code)
143+
}
144+
if err == nil || err.Error() != "authentication failed" {
145+
t.Errorf("expected error 'authentication failed', got %v", err)
146+
}
147+
w.WriteHeader(code)
148+
},
149+
expectedStatusCode: http.StatusUnauthorized,
150+
expectedUser: nil,
151+
expectedBody: "",
152+
},
153+
{
154+
name: "Successful request with optional auth context set",
155+
authCtxKey: authCtxKey{},
156+
optionalAuthCtxKey: optionalAuthCtxKey{},
157+
authHeader: "",
158+
mockAuthenticator: func(_ context.Context, _ string) (*auth.User, error) {
159+
t.Fatal("authenticate should not have been called")
160+
161+
// nolint:nilnil // WONTFIX - should not reach this.
162+
return nil, nil
163+
},
164+
mockErrorFn: func(_ context.Context, _ int, _ http.ResponseWriter, _ error) {
165+
t.Fatal("errorFn should not have been called")
166+
},
167+
expectedStatusCode: http.StatusOK,
168+
expectedBody: "next handler was called",
169+
expectedUser: nil,
170+
},
171+
{
172+
name: "Successful authentication",
173+
authCtxKey: authCtxKey{},
174+
optionalAuthCtxKey: nil,
175+
authHeader: "Bearer my-token",
129176
mockAuthenticator: func(_ context.Context, token string) (*auth.User, error) {
130177
if token != "my-token" {
131178
t.Errorf("expected token 'my-token', got %s", token)
@@ -161,19 +208,14 @@ func TestBearerTokenAuthenticationMiddleware(t *testing.T) {
161208

162209
middleware := NewBearerTokenAuthenticationMiddleware(
163210
&mockBearerTokenAuthenticator{tc.mockAuthenticator},
164-
tc.ctxKey,
211+
tc.authCtxKey,
212+
tc.optionalAuthCtxKey,
165213
tc.mockErrorFn,
166214
)
167215

168216
handler := middleware(nextHandler)
169217

170-
req := httptest.NewRequest(http.MethodGet, "/", nil)
171-
if tc.authHeader != "" {
172-
req.Header.Set("Authorization", tc.authHeader)
173-
}
174-
if tc.ctxKey != nil {
175-
req = req.WithContext(context.WithValue(req.Context(), tc.ctxKey, "authCtxValue"))
176-
}
218+
req := createTestRequest(tc.authHeader, tc.authCtxKey, tc.optionalAuthCtxKey)
177219

178220
rr := httptest.NewRecorder()
179221
handler.ServeHTTP(rr, req)
@@ -196,6 +238,21 @@ func (m *mockBearerTokenAuthenticator) Authenticate(ctx context.Context, token s
196238
return m.authenticateFn(ctx, token)
197239
}
198240

241+
func createTestRequest(authHeader string, authCtxKey any, optionalAuthCtxKey any) *http.Request {
242+
req := httptest.NewRequest(http.MethodGet, "/", nil)
243+
if authHeader != "" {
244+
req.Header.Set("Authorization", authHeader)
245+
}
246+
if authCtxKey != nil {
247+
req = req.WithContext(context.WithValue(req.Context(), authCtxKey, "authCtxValue"))
248+
}
249+
if optionalAuthCtxKey != nil {
250+
req = req.WithContext(context.WithValue(req.Context(), optionalAuthCtxKey, "optionalAuthCtxValue"))
251+
}
252+
253+
return req
254+
}
255+
199256
func assertStatusCode(t *testing.T, rr *httptest.ResponseRecorder, expectedCode int) {
200257
t.Helper()
201258
if rr.Code != expectedCode {

openapi/backend/openapi.yaml

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
openapi: '3.0.2'
15+
openapi: '3.0.4'
1616
info:
1717
title: webstatus.dev API
1818
version: 0.1.0
@@ -569,7 +569,7 @@ paths:
569569
- $ref: '#/components/parameters/paginationTokenParam'
570570
- $ref: '#/components/parameters/paginationSizeParam'
571571
security:
572-
- bearerAuth:
572+
- bearerAuth: []
573573
responses:
574574
'200':
575575
description: OK
@@ -613,7 +613,7 @@ paths:
613613
summary: Get a user's bookmark status
614614
operationId: getUserSavedSearchBookmark
615615
security:
616-
- bearerAuth:
616+
- bearerAuth: []
617617
responses:
618618
'200':
619619
description: OK
@@ -732,7 +732,7 @@ paths:
732732
summary: Create a saved search
733733
operationId: createSavedSearch
734734
security:
735-
- bearerAuth:
735+
- bearerAuth: []
736736
requestBody:
737737
required: true
738738
content:
@@ -781,6 +781,11 @@ paths:
781781
get:
782782
summary: Get saved search
783783
operationId: getSavedSearch
784+
security:
785+
- bearerAuth: []
786+
# User can opt out of providing a bearer token.
787+
# They just won't get bookmark and role information.
788+
- noAuth: []
784789
responses:
785790
'200':
786791
description: OK
@@ -803,7 +808,7 @@ paths:
803808
patch:
804809
operationId: updateSavedSearch
805810
security:
806-
- bearerAuth:
811+
- bearerAuth: []
807812
requestBody:
808813
required: true
809814
content:
@@ -844,7 +849,7 @@ paths:
844849
delete:
845850
operationId: removeSavedSearch
846851
security:
847-
- bearerAuth:
852+
- bearerAuth: []
848853
responses:
849854
'204':
850855
description: No Content (successful deletion)

0 commit comments

Comments
 (0)