Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fetching bearer from cookie #179

Merged
merged 4 commits into from
Mar 21, 2024
Merged

Fix fetching bearer from cookie #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8053877
handlers: fix bearer fetching from cookie
roman-khimov Mar 21, 2024
6eea53f
handlers: simplify code in getPrincipal
roman-khimov Mar 21, 2024
070b440
handlers: unexport bearer prefix/cookie definitions
roman-khimov Mar 21, 2024
3e4bb24
cmd: add some tests for cookie-based bearer
roman-khimov Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions cmd/neofs-rest-gw/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ func runTests(ctx context.Context, t *testing.T, key *keys.PrivateKey, node stri
t.Run("rest get object", func(t *testing.T) { restObjectGet(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest get object unauthenticated", func(t *testing.T) { restObjectGetUnauthenticated(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest get object full bearer", func(t *testing.T) { restObjectGetFullBearer(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest get object with bearer in cookie", func(t *testing.T) { restObjectGetBearerCookie(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest delete object", func(t *testing.T) { restObjectDelete(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest search objects", func(t *testing.T) { restObjectsSearch(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest upload object", func(t *testing.T) { restObjectUpload(ctx, t, clientPool, cnrID, signer) })
t.Run("rest upload object with bearer in cookie", func(t *testing.T) { restObjectUploadCookie(ctx, t, clientPool, cnrID, signer) })
t.Run("rest head object", func(t *testing.T) { restObjectHead(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest head by attribute", func(t *testing.T) { restObjectHeadByAttribute(ctx, t, clientPool, &owner, cnrID, signer) })
t.Run("rest get by attribute", func(t *testing.T) { restObjectGetByAttribute(ctx, t, clientPool, &owner, cnrID, signer) })
Expand Down Expand Up @@ -1084,6 +1086,12 @@ func restObjectGetUnauthenticated(ctx context.Context, t *testing.T, p *pool.Poo
}

func restObjectGetFullBearer(ctx context.Context, t *testing.T, p *pool.Pool, ownerID *user.ID, cnrID cid.ID, signer user.Signer) {
restObjectGetWithBearer(ctx, t, p, ownerID, cnrID, signer, false)
}
func restObjectGetBearerCookie(ctx context.Context, t *testing.T, p *pool.Pool, ownerID *user.ID, cnrID cid.ID, signer user.Signer) {
restObjectGetWithBearer(ctx, t, p, ownerID, cnrID, signer, true)
}
func restObjectGetWithBearer(ctx context.Context, t *testing.T, p *pool.Pool, ownerID *user.ID, cnrID cid.ID, signer user.Signer, cookie bool) {
content := []byte("some content")
attributes := map[string]string{
object.AttributeFileName: "get-obj-name",
Expand Down Expand Up @@ -1140,7 +1148,11 @@ func restObjectGetFullBearer(ctx context.Context, t *testing.T, p *pool.Pool, ow

request, err = http.NewRequest(http.MethodGet, testHost+"/v1/objects/"+cnrID.EncodeToString()+"/"+objID.EncodeToString()+"?"+query.Encode(), nil)
require.NoError(t, err)
request.Header.Add("Authorization", "Bearer "+resp.Token)
if cookie {
request.Header.Add("Cookie", "Bearer="+resp.Token+";")
} else {
request.Header.Add("Authorization", "Bearer "+resp.Token)
}

objInfo := &apiserver.ObjectInfo{}
doRequest(t, httpClient, request, http.StatusOK, objInfo)
Expand Down Expand Up @@ -1767,6 +1779,12 @@ func restBalance(_ context.Context, t *testing.T) {
}

func restObjectUpload(ctx context.Context, t *testing.T, clientPool *pool.Pool, cnrID cid.ID, signer user.Signer) {
restObjectUploadInt(ctx, t, clientPool, cnrID, signer, false)
}
func restObjectUploadCookie(ctx context.Context, t *testing.T, clientPool *pool.Pool, cnrID cid.ID, signer user.Signer) {
restObjectUploadInt(ctx, t, clientPool, cnrID, signer, true)
}
func restObjectUploadInt(ctx context.Context, t *testing.T, clientPool *pool.Pool, cnrID cid.ID, signer user.Signer, cookie bool) {
bt := apiserver.Bearer{
Object: []apiserver.Record{{
Operation: apiserver.OperationPUT,
Expand Down Expand Up @@ -1817,8 +1835,11 @@ func restObjectUpload(ctx context.Context, t *testing.T, clientPool *pool.Pool,
require.NoError(t, err)

request.Header.Set("Content-Type", writer.FormDataContentType())
request.Header.Add("Authorization", "Bearer "+base64.StdEncoding.EncodeToString(actualTokenRaw))

if cookie {
request.Header.Add("Cookie", "Bearer="+base64.StdEncoding.EncodeToString(actualTokenRaw)+";")
} else {
request.Header.Add("Authorization", "Bearer "+base64.StdEncoding.EncodeToString(actualTokenRaw))
}
addr := &apiserver.AddressForUpload{}
doRequest(t, httpClient, request, http.StatusOK, addr)

Expand Down
32 changes: 10 additions & 22 deletions handlers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@
}

const (
// BearerPrefix is the prefix for authorization token.
BearerPrefix = "Bearer "
BearerCookiePrefix = "Bearer="
// bearerCookieName is the name of the bearer cookie.
bearerCookieName = "Bearer"
// bearerPrefix is the prefix for authorization token.
bearerPrefix = bearerCookieName + " "

accessControlAllowOriginHeader = "Access-Control-Allow-Origin"
authorizationHeader = "Authorization"
Expand Down Expand Up @@ -78,29 +79,25 @@
return "", nil
}

if !strings.HasPrefix(headerValue, BearerPrefix) {
if !strings.HasPrefix(headerValue, bearerPrefix) {

Check warning on line 82 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L82 

Added line #L82 was not covered by tests
return "", errors.New("http auth: no bearer token")
}

if headerValue = strings.TrimPrefix(headerValue, BearerPrefix); len(headerValue) == 0 {
if headerValue = strings.TrimPrefix(headerValue, bearerPrefix); len(headerValue) == 0 {

Check warning on line 86 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L86 

Added line #L86 was not covered by tests
return "", errors.New("http auth: bearer token is empty")
}

return headerValue, nil
}

func getPrincipalFromCookie(ctx echo.Context) (string, error) {
var bearerCookie string

for _, cookie := range ctx.Request().Cookies() {
cookieValue := strings.TrimSpace(cookie.Value)
if strings.HasPrefix(cookieValue, BearerCookiePrefix) {
bearerCookie = strings.TrimPrefix(cookieValue, BearerCookiePrefix)
if len(bearerCookie) == 0 {
if cookie.Name == bearerCookieName {
if len(cookie.Value) == 0 {

Check warning on line 96 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L95-L96 

Added lines #L95 - L96 were not covered by tests
return "", errors.New("cookie auth: bearer token is empty")
}

return bearerCookie, nil
return cookie.Value, nil

Check warning on line 100 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L100 

Added line #L100 was not covered by tests
}
}

Expand All @@ -117,16 +114,7 @@
return principal, nil
}

principal, err = getPrincipalFromCookie(ctx)
if err != nil {
return "", err
}

if principal != "" {
return principal, nil
}

return "", nil
return getPrincipalFromCookie(ctx)

Check warning on line 117 in handlers/api.go

View check run for this annotation

Codecov / codecov/patch

handlers/api.go#L117 

Added line #L117 was not covered by tests
}

func (a *RestAPI) logAndGetErrorResponse(msg string, err error, fields ...zap.Field) *apiserver.ErrorResponse {
Expand Down
Loading