Skip to content

Commit

Permalink
Ignore auth for /swagger.json endpoint (#1377)
Browse files Browse the repository at this point in the history
* Ignore authentication for `/swagger.json` in ReBAC admin API

Signed-off-by: Babak K. Shandiz <[email protected]>

* Add test case to verify skipping auth for `/swagger.json`

Signed-off-by: Babak K. Shandiz <[email protected]>

* Fix ReBAC Admin test

Signed-off-by: Babak K. Shandiz <[email protected]>

* Use a map to lookup for endpoints

Signed-off-by: Babak K. Shandiz <[email protected]>

* Upgrade `rebac-admin-ui-handlers` to `v0.1.2`

Signed-off-by: Babak K. Shandiz <[email protected]>

* Use zero-length struct in map/set declaration

Signed-off-by: Babak K. Shandiz <[email protected]>

---------

Signed-off-by: Babak K. Shandiz <[email protected]>
  • Loading branch information
babakks authored Sep 25, 2024
1 parent c8fd095 commit 85c03a9
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cmd/jimmsrv/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func NewService(ctx context.Context, p Params) (*Service, error) {

s.mux.Mount("/metrics", promhttp.Handler())

s.mux.Mount("/rebac", middleware.AuthenticateRebac(rebacBackend.Handler(""), &s.jimm))
s.mux.Mount("/rebac", middleware.AuthenticateRebac("/rebac", rebacBackend.Handler(""), &s.jimm))

mountHandler(
"/debug",
Expand Down
5 changes: 4 additions & 1 deletion cmd/jimmsrv/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,10 @@ func TestRebacAdminApi(t *testing.T) {
response, err := srv.Client().Get(srv.URL + "/rebac/v1/swagger.json")
c.Assert(err, qt.IsNil)
defer response.Body.Close()
c.Assert(response.StatusCode, qt.Equals, 401)

// The `/swagger.json` endpoint doesn't require authentication, so the returned
// status code should be 200.
c.Assert(response.StatusCode, qt.Equals, 200)
}

func TestThirdPartyCaveatDischarge(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ require (
require (
github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a
github.com/canonical/ofga v0.10.0
github.com/canonical/rebac-admin-ui-handlers v0.1.1
github.com/canonical/rebac-admin-ui-handlers v0.1.2
github.com/coreos/go-oidc/v3 v3.9.0
github.com/dustinkirkland/golang-petname v0.0.0-20231002161417-6a283f1aaaf2
github.com/go-chi/chi/v5 v5.0.12
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ github.com/canonical/rebac-admin-ui-handlers v0.1.0 h1:Bef1N/RgQine8hHX4ZMksQz/1
github.com/canonical/rebac-admin-ui-handlers v0.1.0/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k=
github.com/canonical/rebac-admin-ui-handlers v0.1.1 h1:rjsb45diShhwD/uUFpai6gmhFUzT+jTdsnEWcOvcKx4=
github.com/canonical/rebac-admin-ui-handlers v0.1.1/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k=
github.com/canonical/rebac-admin-ui-handlers v0.1.2 h1:tQU/NWQVD9IEgy3ct8JWZXpLK/dedWjKcy9E8W4glpo=
github.com/canonical/rebac-admin-ui-handlers v0.1.2/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k=
github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs=
github.com/cenkalti/backoff/v3 v3.2.2 h1:cfUAAO3yvKMYKPrvhDuHSwQnhZNk/RMHKdZqKTxfm6M=
github.com/cenkalti/backoff/v3 v3.2.2/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs=
Expand Down
27 changes: 22 additions & 5 deletions internal/middleware/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package middleware

import (
"net/http"
"strings"

rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1"
"github.com/juju/zaputil/zapctx"
Expand All @@ -27,11 +28,18 @@ func AuthenticateViaCookie(next http.Handler, jimm jujuapi.JIMM) http.Handler {
})
}

// AuthenticateRebac is a layer on top of AuthenticateViaCookie
// It places the OpenFGA user for the session identity inside the request's context
// and verifies that the user is a JIMM admin.
func AuthenticateRebac(next http.Handler, jimm jujuapi.JIMM) http.Handler {
return AuthenticateViaCookie(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Set of ReBAC Admin API endpoints that do not require authentication.
var unauthenticatedEndpoints = map[string]struct{}{
"/v1/swagger.json": {},
}

// AuthenticateRebac is a layer on top of AuthenticateViaCookie. It places the
// OpenFGA user for the session identity inside the request's context and
// verifies that the user is a JIMM admin. Note that the method needs the base
// URL to decide if the request does not require authentication; this is to
// safeguard against conflicting/similar endpoint names in the future.
func AuthenticateRebac(baseURL string, next http.Handler, jimm jujuapi.JIMM) http.Handler {
cookieAuthenticator := AuthenticateViaCookie(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()

identity := auth.SessionIdentityFromContext(ctx)
Expand All @@ -56,4 +64,13 @@ func AuthenticateRebac(next http.Handler, jimm jujuapi.JIMM) http.Handler {
ctx = rebac_handlers.ContextWithIdentity(ctx, user)
next.ServeHTTP(w, r.WithContext(ctx))
}), jimm)

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
relativePath, _ := strings.CutPrefix(r.URL.Path, baseURL)
if _, found := unauthenticatedEndpoints[relativePath]; found {
next.ServeHTTP(w, r)
return
}
cookieAuthenticator.ServeHTTP(w, r)
})
}
65 changes: 52 additions & 13 deletions internal/middleware/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package middleware_test
import (
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -22,12 +23,18 @@ import (

// Checks if the authenticator responsible for access control to rebac admin handlers works correctly.
func TestAuthenticateRebac(t *testing.T) {
c := qt.New(t)
baseURL := "/rebac"

testUser := "[email protected]"
tests := []struct {
name string
setupRequest func() *http.Request
setupHandler func() http.Handler
mockAuthBrowserSession func(ctx context.Context, w http.ResponseWriter, req *http.Request) (context.Context, error)
jimmAdmin bool
expectedStatus int
expectedBody string
}{
{
name: "success",
Expand Down Expand Up @@ -59,12 +66,26 @@ func TestAuthenticateRebac(t *testing.T) {
jimmAdmin: false,
expectedStatus: http.StatusUnauthorized,
},
{
name: "should skip auth for /swagger.json",
setupRequest: func() *http.Request {
req, _ := http.NewRequest(http.MethodGet, baseURL+"/v1/swagger.json", nil)
return req
},
setupHandler: func() http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("the-body"))
})
},
jimmAdmin: false,
expectedStatus: http.StatusOK,
expectedBody: "the-body",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := qt.New(t)

c.Run(tt.name, func(c *qt.C) {
j := jimmtest.JIMM{
LoginService: mocks.LoginService{
AuthenticateBrowserSession_: func(ctx context.Context, w http.ResponseWriter, req *http.Request) (context.Context, error) {
Expand All @@ -76,23 +97,41 @@ func TestAuthenticateRebac(t *testing.T) {
return &openfga.User{Identity: &user, JimmAdmin: tt.jimmAdmin}, nil
},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)

var req *http.Request
if tt.setupRequest != nil {
req = tt.setupRequest()
} else {
req = httptest.NewRequest(http.MethodGet, baseURL, nil)
}

w := httptest.NewRecorder()

handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
identity, err := rebac_handlers.GetIdentityFromContext(r.Context())
c.Assert(err, qt.IsNil)
var handler http.Handler
if tt.setupHandler != nil {
handler = tt.setupHandler()
} else {
handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
identity, err := rebac_handlers.GetIdentityFromContext(r.Context())
c.Assert(err, qt.IsNil)

user, ok := identity.(*openfga.User)
c.Assert(ok, qt.IsTrue)
c.Assert(user.Name, qt.Equals, testUser)
user, ok := identity.(*openfga.User)
c.Assert(ok, qt.IsTrue)
c.Assert(user.Name, qt.Equals, testUser)

w.WriteHeader(http.StatusOK)
})
middleware := middleware.AuthenticateRebac(handler, &j)
w.WriteHeader(http.StatusOK)
})
}

middleware := middleware.AuthenticateRebac(baseURL, handler, &j)
middleware.ServeHTTP(w, req)

c.Assert(w.Code, qt.Equals, tt.expectedStatus)
if tt.expectedBody != "" {
body, err := io.ReadAll(w.Body)
c.Assert(err, qt.IsNil)
c.Assert(string(body), qt.Equals, tt.expectedBody)
}
})
}
}

0 comments on commit 85c03a9

Please sign in to comment.