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.

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

filters/auth: add grant cookie encoder #2953

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 13 additions & 22 deletions filters/auth/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,10 @@ func loginRedirectWithOverride(ctx filters.FilterContext, config *OAuthConfig, o
})
}

func (f *grantFilter) refreshToken(c *cookie, req *http.Request) (*oauth2.Token, error) {
func (f *grantFilter) refreshToken(token *oauth2.Token, req *http.Request) (*oauth2.Token, error) {
// Set the expiry of the token to the past to trigger oauth2.TokenSource
// to refresh the access token.
token := &oauth2.Token{
AccessToken: c.AccessToken,
RefreshToken: c.RefreshToken,
Expiry: time.Now().Add(-time.Minute),
}
token.Expiry = time.Now().Add(-time.Minute)

ctx := providerContext(f.config)

Expand All @@ -114,12 +110,12 @@ func (f *grantFilter) refreshToken(c *cookie, req *http.Request) (*oauth2.Token,
return tokenSource.Token()
}

func (f *grantFilter) refreshTokenIfRequired(c *cookie, ctx filters.FilterContext) (*oauth2.Token, error) {
canRefresh := c.RefreshToken != ""
func (f *grantFilter) refreshTokenIfRequired(t *oauth2.Token, ctx filters.FilterContext) (*oauth2.Token, error) {
canRefresh := t.RefreshToken != ""

if c.isAccessTokenExpired() {
if time.Now().After(t.Expiry) {
if canRefresh {
token, err := f.refreshToken(c, ctx.Request())
token, err := f.refreshToken(t, ctx.Request())
if err == nil {
// Remember that this token was just successfully refreshed
// so that we can send an updated cookie in the response.
Expand All @@ -130,12 +126,7 @@ func (f *grantFilter) refreshTokenIfRequired(c *cookie, ctx filters.FilterContex
return nil, errExpiredToken
}
} else {
return &oauth2.Token{
AccessToken: c.AccessToken,
TokenType: "Bearer",
RefreshToken: c.RefreshToken,
Expiry: c.Expiry,
}, nil
return t, nil
}
}

Expand Down Expand Up @@ -179,15 +170,13 @@ func (f *grantFilter) setupToken(token *oauth2.Token, tokeninfo map[string]inter
}

func (f *grantFilter) Request(ctx filters.FilterContext) {
req := ctx.Request()

c, err := extractCookie(req, f.config)
token, err := f.config.GrantCookieEncoder.Read(ctx.Request())
if err == http.ErrNoCookie {
loginRedirect(ctx, f.config)
return
}

token, err := f.refreshTokenIfRequired(c, ctx)
token, err = f.refreshTokenIfRequired(token, ctx)
if err != nil {
// Refresh failed and we no longer have a valid access token.
loginRedirect(ctx, f.config)
Expand Down Expand Up @@ -221,11 +210,13 @@ func (f *grantFilter) Response(ctx filters.FilterContext) {
return
}

c, err := createCookie(f.config, ctx.Request().Host, token)
cookies, err := f.config.GrantCookieEncoder.Update(ctx.Request(), token)
if err != nil {
ctx.Logger().Errorf("Failed to generate cookie: %v.", err)
return
}

ctx.Response().Header.Add("Set-Cookie", c.String())
for _, c := range cookies {
ctx.Response().Header.Add("Set-Cookie", c.String())
}
}
13 changes: 8 additions & 5 deletions filters/auth/grantcallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,23 @@ func (f *grantCallbackFilter) Request(ctx filters.FilterContext) {
return
}

c, err := createCookie(f.config, req.Host, token)
cookies, err := f.config.GrantCookieEncoder.Update(req, token)
if err != nil {
ctx.Logger().Errorf("Failed to create OAuth grant cookie: %v.", err)
serverError(ctx)
return
}

ctx.Serve(&http.Response{
resp := &http.Response{
StatusCode: http.StatusTemporaryRedirect,
Header: http.Header{
"Location": []string{state.RequestURL},
"Set-Cookie": []string{c.String()},
"Location": []string{state.RequestURL},
},
})
}
for _, c := range cookies {
resp.Header.Add("Set-Cookie", c.String())
}
ctx.Serve(resp)
}

func (f *grantCallbackFilter) Response(ctx filters.FilterContext) {}
16 changes: 16 additions & 0 deletions filters/auth/grantconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ type OAuthConfig struct {
// GrantTokeninfoKeys, optional. When not empty, keys not in this list are removed from the tokeninfo map.
GrantTokeninfoKeys []string

// GrantCookieEncoder, optional. Cookie encoder stores and extracts OAuth token from cookies.
GrantCookieEncoder CookieEncoder

// TokeninfoSubjectKey, optional. When set, it is used to look up the subject
// ID in the tokeninfo map received from a tokeninfo endpoint request.
TokeninfoSubjectKey string
Expand Down Expand Up @@ -257,6 +260,19 @@ func (c *OAuthConfig) Init() error {
}
}

if c.GrantCookieEncoder == nil {
encryption, err := c.Secrets.GetEncrypter(secretsRefreshInternal, c.SecretFile)
if err != nil {
return err
}
c.GrantCookieEncoder = &EncryptedCookieEncoder{
Encryption: encryption,
CookieName: c.TokenCookieName,
RemoveSubdomains: *c.TokenCookieRemoveSubdomains,
Insecure: c.Insecure,
}
}

c.initialized = true
return nil
}
Expand Down
89 changes: 60 additions & 29 deletions filters/auth/grantcookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,74 @@ import (
"golang.org/x/oauth2"
)

type CookieEncoder interface {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want cookie encoder or a generic token encoder that works with http.Response and may encode/extract token into/from cookie or e.g. headers?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing Oauth2 approach here is designed to work with a cookie. Cookie-specific is fine for us unless you think it can be used somewhere else

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AtorVdm I've updated CookieEncoder interface, please check if its suitable for the implementation you have in mind.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a discussion with @AlexanderYastrebov. This works for us! If this is left as-is - we will implement our own compression on top. Another option is if you can split the cookie into multiple ones based on cookie max size param (can be defined). Either way this PR would be a prerequisite for this so I think it's good to get it merged.

// Update creates a set of cookies that encodes the token and deletes previously existing cookies if necessary.
// When token is nil it only returns cookies to delete.
Update(request *http.Request, token *oauth2.Token) ([]*http.Cookie, error)

// Read extracts the token from the request cookies.
Read(request *http.Request) (*oauth2.Token, error)
}

// EncryptedCookieEncoder is a CookieEncoder that encrypts the token before storing it in a cookie.
type EncryptedCookieEncoder struct {
Encryption secrets.Encryption
CookieName string
RemoveSubdomains int
Insecure bool
}

var _ CookieEncoder = &EncryptedCookieEncoder{}

func (ce *EncryptedCookieEncoder) Update(request *http.Request, token *oauth2.Token) ([]*http.Cookie, error) {
if token != nil {
c, err := ce.createCookie(request.Host, token)
if err != nil {
return nil, err
}
return []*http.Cookie{c}, nil
} else {
c := ce.createDeleteCookie(request.Host)
return []*http.Cookie{c}, nil
}
}

func (ce *EncryptedCookieEncoder) Read(request *http.Request) (*oauth2.Token, error) {
c, err := ce.extractCookie(request)
if err != nil {
return nil, err
}

return &oauth2.Token{
AccessToken: c.AccessToken,
TokenType: "Bearer",
RefreshToken: c.RefreshToken,
Expiry: c.Expiry,
}, nil
}

type cookie struct {
AccessToken string `json:"access_token"`
RefreshToken string `json:"refresh_token"`
Expiry time.Time `json:"expiry,omitempty"`
Domain string `json:"domain,omitempty"`
}

func decodeCookie(cookieHeader string, config *OAuthConfig) (c *cookie, err error) {
func (ce *EncryptedCookieEncoder) decodeCookie(cookieHeader string) (c *cookie, err error) {
var eb []byte
if eb, err = base64.StdEncoding.DecodeString(cookieHeader); err != nil {
return
}

var encryption secrets.Encryption
if encryption, err = config.Secrets.GetEncrypter(secretsRefreshInternal, config.SecretFile); err != nil {
return
}

var b []byte
if b, err = encryption.Decrypt(eb); err != nil {
if b, err = ce.Encryption.Decrypt(eb); err != nil {
return
}

err = json.Unmarshal(b, &c)
return
}

func (c *cookie) isAccessTokenExpired() bool {
now := time.Now()
return now.After(c.Expiry)
}

// allowedForHost checks if provided host matches cookie domain
// according to https://www.rfc-editor.org/rfc/rfc6265#section-5.1.3
func (c *cookie) allowedForHost(host string) bool {
Expand All @@ -61,14 +97,14 @@ func (c *cookie) allowedForHost(host string) bool {
// cookie of the same name.
// The grant token cookie is extracted so it does not get exposed to untrusted downstream
// services.
func extractCookie(request *http.Request, config *OAuthConfig) (*cookie, error) {
func (ce *EncryptedCookieEncoder) extractCookie(request *http.Request) (*cookie, error) {
cookies := request.Cookies()
for i, c := range cookies {
if c.Name != config.TokenCookieName {
if c.Name != ce.CookieName {
continue
}

decoded, err := decodeCookie(c.Value, config)
decoded, err := ce.decodeCookie(c.Value)
if err == nil && decoded.allowedForHost(request.Host) {
request.Header.Del("Cookie")
for j, c := range cookies {
Expand All @@ -84,20 +120,20 @@ func extractCookie(request *http.Request, config *OAuthConfig) (*cookie, error)

// createDeleteCookie creates a cookie, which instructs the client to clear the grant
// token cookie when used with a Set-Cookie header.
func createDeleteCookie(config *OAuthConfig, host string) *http.Cookie {
func (ce *EncryptedCookieEncoder) createDeleteCookie(host string) *http.Cookie {
return &http.Cookie{
Name: config.TokenCookieName,
Name: ce.CookieName,
Value: "",
Path: "/",
Domain: extractDomainFromHost(host, *config.TokenCookieRemoveSubdomains),
Domain: extractDomainFromHost(host, ce.RemoveSubdomains),
MaxAge: -1,
Secure: !config.Insecure,
Secure: !ce.Insecure,
HttpOnly: true,
}
}

func createCookie(config *OAuthConfig, host string, t *oauth2.Token) (*http.Cookie, error) {
domain := extractDomainFromHost(host, *config.TokenCookieRemoveSubdomains)
func (ce *EncryptedCookieEncoder) createCookie(host string, t *oauth2.Token) (*http.Cookie, error) {
domain := extractDomainFromHost(host, ce.RemoveSubdomains)
c := &cookie{
AccessToken: t.AccessToken,
RefreshToken: t.RefreshToken,
Expand All @@ -110,12 +146,7 @@ func createCookie(config *OAuthConfig, host string, t *oauth2.Token) (*http.Cook
return nil, err
}

encryption, err := config.Secrets.GetEncrypter(secretsRefreshInternal, config.SecretFile)
if err != nil {
return nil, err
}

eb, err := encryption.Encrypt(b)
eb, err := ce.Encryption.Encrypt(b)
if err != nil {
return nil, err
}
Expand All @@ -128,12 +159,12 @@ func createCookie(config *OAuthConfig, host string, t *oauth2.Token) (*http.Cook
// Since we don't know the actual refresh token expiry, set it to
// 30 days as a good compromise.
return &http.Cookie{
Name: config.TokenCookieName,
Name: ce.CookieName,
Value: b64,
Path: "/",
Domain: domain,
Expires: t.Expiry.Add(time.Hour * 24 * 30),
Secure: !config.Insecure,
Secure: !ce.Insecure,
HttpOnly: true,
}, nil
}
5 changes: 2 additions & 3 deletions filters/auth/grantcookie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ const (
)

func newGrantCookies(t *testing.T, config *OAuthConfig, host string, token oauth2.Token) []*http.Cookie {
cookie, err := createCookie(config, host, &token)
cookies, err := config.GrantCookieEncoder.Update(&http.Request{Host: host}, &token)
require.NoError(t, err)

return []*http.Cookie{cookie}
return cookies
}

func NewGrantCookies(t *testing.T, config *OAuthConfig) []*http.Cookie {
Expand Down
22 changes: 14 additions & 8 deletions filters/auth/grantlogout.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (f *grantLogoutFilter) Request(ctx filters.FilterContext) {

req := ctx.Request()

c, err := extractCookie(req, f.config)
token, err := f.config.GrantCookieEncoder.Read(req)
if err != nil {
unauthorized(
ctx,
Expand All @@ -136,7 +136,7 @@ func (f *grantLogoutFilter) Request(ctx filters.FilterContext) {
return
}

if c.AccessToken == "" && c.RefreshToken == "" {
if token.AccessToken == "" && token.RefreshToken == "" {
unauthorized(
ctx,
"",
Expand All @@ -153,15 +153,15 @@ func (f *grantLogoutFilter) Request(ctx filters.FilterContext) {
}

var accessTokenRevokeError, refreshTokenRevokeError error
if c.AccessToken != "" {
accessTokenRevokeError = f.revokeTokenType(authConfig, accessTokenType, c.AccessToken)
if token.AccessToken != "" {
accessTokenRevokeError = f.revokeTokenType(authConfig, accessTokenType, token.AccessToken)
if accessTokenRevokeError != nil {
ctx.Logger().Errorf("%v", accessTokenRevokeError)
}
}

if c.RefreshToken != "" {
refreshTokenRevokeError = f.revokeTokenType(authConfig, refreshTokenType, c.RefreshToken)
if token.RefreshToken != "" {
refreshTokenRevokeError = f.revokeTokenType(authConfig, refreshTokenType, token.RefreshToken)
if refreshTokenRevokeError != nil {
ctx.Logger().Errorf("%v", refreshTokenRevokeError)
}
Expand All @@ -173,6 +173,12 @@ func (f *grantLogoutFilter) Request(ctx filters.FilterContext) {
}

func (f *grantLogoutFilter) Response(ctx filters.FilterContext) {
deleteCookie := createDeleteCookie(f.config, ctx.Request().Host)
ctx.Response().Header.Add("Set-Cookie", deleteCookie.String())
cookies, err := f.config.GrantCookieEncoder.Update(ctx.Request(), nil)
if err != nil {
ctx.Logger().Errorf("Failed to delete cookies: %v.", err)
return
}
for _, c := range cookies {
ctx.Response().Header.Add("Set-Cookie", c.String())
}
}
Loading
Loading