Skip to content

Commit

Permalink
fix: do not use secure cookies if protocol is http
Browse files Browse the repository at this point in the history
  • Loading branch information
@imba28
imba28 committed Jul 12, 2022
1 parent 09e73c0 commit 28844ca
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 40 deletions.
52 changes: 19 additions & 33 deletions pkg/api/auth.go
View file
Expand Up @@ -123,6 +123,17 @@ type AuthApiService struct {
publicHttpPort int
}

func (a AuthApiService) cookie(name, value, path string) http.Cookie {
return http.Cookie{
Name: name,
Path: path,
Value: value,
Secure: a.publicHttpProtocol == "https",
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
}
}

func (a AuthApiService) jwtTokenHeaders(t oauth2.Token, generateRefreshToken bool) (map[string][]string, error) {
headers := make(map[string][]string)
var cookies []string
Expand All @@ -131,14 +142,7 @@ func (a AuthApiService) jwtTokenHeaders(t oauth2.Token, generateRefreshToken boo
if err != nil {
return nil, errors.New("could not sign access jwt")
}
accessTokenCookie := http.Cookie{
Name: "jwt",
Path: "/api",
Value: accessToken,
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
}
accessTokenCookie := a.cookie("jwt", accessToken, "/api")
cookies = append(cookies, accessTokenCookie.String())

if generateRefreshToken {
Expand All @@ -147,15 +151,8 @@ func (a AuthApiService) jwtTokenHeaders(t oauth2.Token, generateRefreshToken boo
if err != nil {
return nil, errors.New("could not sign refresh jwt")
}
refreshTokenCookie := http.Cookie{
Name: "jwt-refresh",
Path: "/api/auth",
Value: refreshToken,
Expires: time.Now().Add(time.Hour * 24),
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
}
refreshTokenCookie := a.cookie("jwt-refresh", refreshToken, "/api/auth")
refreshTokenCookie.Expires = time.Now().Add(refreshTokenExpiry)
cookies = append(cookies, refreshTokenCookie.String())
}

Expand All @@ -165,23 +162,12 @@ func (a AuthApiService) jwtTokenHeaders(t oauth2.Token, generateRefreshToken boo
}

func (a AuthApiService) AuthLogoutGet(ctx context.Context) (openapi.ImplResponse, error) {
accessTokenCookie := http.Cookie{
Name: "jwt",
Path: "/api",
Expires: time.Unix(0, 0),
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
}
accessTokenCookie := a.cookie("jwt", "", "/api")
accessTokenCookie.Expires = time.Unix(0, 0)

// todo: revoke refresh token => delete from database
refreshTokenCookie := http.Cookie{
Name: "jwt-refresh",
Path: "/api/auth/refresh",
Expires: time.Unix(0, 0),
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteNoneMode,
}
refreshTokenCookie := a.cookie("jwt-refresh", "", "/api/auth/")
refreshTokenCookie.Expires = time.Unix(0, 0)

headers := make(map[string][]string)
headers["Set-Cookie"] = []string{accessTokenCookie.String(), refreshTokenCookie.String()}
Expand Down
87 changes: 80 additions & 7 deletions pkg/api/auth_test.go
View file
Expand Up @@ -140,6 +140,78 @@ func TestAuthenticationMiddleware(t *testing.T) {
})
}

func TestAuthApiService_AuthLoginPost_secure_cookies(t *testing.T) {
t.Run("if protocol is http do not use secure cookies", func(t *testing.T) {
httpmock.Activate()
defer httpmock.DeactivateAndReset()

httpmock.RegisterResponder("GET", "=~/me$",
func(req *http.Request) (*http.Response, error) {
return httpmock.NewJsonResponse(200, map[string]interface{}{
"email": "user@test.com",
})
},
)

httpmock.RegisterResponder("POST", "=~/api/token",
func(req *http.Request) (*http.Response, error) {
return httpmock.NewJsonResponse(200, map[string]interface{}{
"access_token": "access-token",
"token_type": "Bearer",
"refresh_token": "refresh_token",
})
},
)

ctx := context.Background()
loginRequest := openapi.AuthLoginPostRequest{Code: "oauth-code"}

auth := AuthApiService{
publicHttpProtocol: "http",
}
res, _ := auth.AuthLoginPost(ctx, loginRequest)

cookies := parseCookies(res.Headers["Set-Cookie"])
assert.False(t, cookies[0].Secure)
assert.False(t, cookies[1].Secure)
})

t.Run("if protocol is https define cookies as secure", func(t *testing.T) {
httpmock.Activate()
defer httpmock.DeactivateAndReset()

httpmock.RegisterResponder("GET", "=~/me$",
func(req *http.Request) (*http.Response, error) {
return httpmock.NewJsonResponse(200, map[string]interface{}{
"email": "user@test.com",
})
},
)

httpmock.RegisterResponder("POST", "=~/api/token",
func(req *http.Request) (*http.Response, error) {
return httpmock.NewJsonResponse(200, map[string]interface{}{
"access_token": "access-token",
"token_type": "Bearer",
"refresh_token": "refresh_token",
})
},
)

ctx := context.Background()
loginRequest := openapi.AuthLoginPostRequest{Code: "oauth-code"}

auth := AuthApiService{
publicHttpProtocol: "https",
}
res, _ := auth.AuthLoginPost(ctx, loginRequest)

cookies := parseCookies(res.Headers["Set-Cookie"])
assert.True(t, cookies[0].Secure)
assert.True(t, cookies[1].Secure)
})
}

func TestAuthApiService_AuthLoginPost(t *testing.T) {
httpmock.Activate()
defer httpmock.DeactivateAndReset()
Expand Down Expand Up @@ -171,8 +243,12 @@ func TestAuthApiService_AuthLoginPost(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, http.StatusOK, res.Code)
assert.Len(t, res.Headers["Set-Cookie"], 2, "should set two cookies ")
assert.Contains(t, res.Headers["Set-Cookie"][0], "jwt=", "should set the cookie `jwt` ")
assert.Contains(t, res.Headers["Set-Cookie"][1], "jwt-refresh=", "should set the cookie `jwt-refresh` ")
c := parseCookies(res.Headers["Set-Cookie"])

assert.Equal(t, "jwt", c[0].Name, "should set the cookie `jwt`")
assert.Equal(t, "/api", c[0].Path, "`jwt` should be valid for path /api")
assert.Equal(t, "jwt-refresh", c[1].Name, "should set the cookie `jwt-refresh`")
assert.Equal(t, "/api/auth", c[1].Path, "`jwt-refresh` should only be valid for path /api/auth")
}

func TestAuthApiService_AuthLogoutGet(t *testing.T) {
Expand All @@ -182,9 +258,6 @@ func TestAuthApiService_AuthLogoutGet(t *testing.T) {

assert.Nil(t, err)
assert.Len(t, res.Headers["Set-Cookie"], 2, "should set two cookies")
assert.Contains(t, res.Headers["Set-Cookie"][0], "jwt=", "should set the cookie `jwt` ")
assert.Contains(t, res.Headers["Set-Cookie"][1], "jwt-refresh=", "should set the cookie `jwt-refresh` ")

c := parseCookies(res.Headers["Set-Cookie"])
assert.True(t, c[0].Expires.Before(time.Now()), "jwt cookie should be expired")
assert.True(t, c[1].Expires.Before(time.Now()), "jwt-refresh cookie should be expired")
Expand All @@ -193,8 +266,8 @@ func TestAuthApiService_AuthLogoutGet(t *testing.T) {
func parseCookies(cookieHeaders []string) []*http.Cookie {
header := http.Header{}
for i := range cookieHeaders {
header.Add("Cookie", cookieHeaders[i])
header.Add("Set-Cookie", cookieHeaders[i])
}
req := http.Request{Header: header}
req := http.Response{Header: header}
return req.Cookies()
}

0 comments on commit 28844ca

Please sign in to comment.