Skip to content

Commit

Permalink
fix: wip auth/cookie fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
markphelps committed Jan 16, 2023
1 parent b75cdf1 commit ea1b732
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 5 deletions.
22 changes: 22 additions & 0 deletions internal/config/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"fmt"
"net/url"
"strings"
"time"

Expand Down Expand Up @@ -106,11 +107,32 @@ func (c *AuthenticationConfig) validate() error {
err := errFieldWrap("authentication.session.domain", errValidationRequired)
return fmt.Errorf("when session compatible auth method enabled: %w", err)
}
fmt.Println("session domain: ", c.Session.Domain)
host, err := getHostname(c.Session.Domain)
if err != nil {
return fmt.Errorf("invalid domain: %w", err)
}

// strip scheme and port from domain
// domain cookies are not allowed to have a scheme or port
// https:/golang/go/issues/28297
c.Session.Domain = host
}

return nil
}

func getHostname(rawurl string) (string, error) {
if !strings.Contains(rawurl, "://") {
rawurl = "http://" + rawurl
}
u, err := url.Parse(rawurl)
if err != nil {
return "", err
}
return strings.Split(u.Host, ":")[0], nil
}

// AuthenticationSession configures the session produced for browsers when
// establishing authentication via HTTP.
type AuthenticationSession struct {
Expand Down
15 changes: 10 additions & 5 deletions internal/server/auth/method/oidc/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,9 @@ func (m Middleware) Handler(next http.Handler) http.Handler {
query.Set("state", encoded)
r.URL.RawQuery = query.Encode()

http.SetCookie(w, &http.Cookie{
Name: stateCookieKey,
Value: encoded,
Domain: m.Config.Domain,
cookie := &http.Cookie{
Name: stateCookieKey,
Value: encoded,
// bind state cookie to provider callback
Path: "/auth/v1/method/oidc/" + provider + "/callback",
Expires: time.Now().Add(m.Config.StateLifetime),
Expand All @@ -134,7 +133,13 @@ func (m Middleware) Handler(next http.Handler) http.Handler {
// we need to support cookie forwarding when user
// is being navigated from authorizing server
SameSite: http.SameSiteLaxMode,
})
}

if m.Config.Domain != "localhost" {
cookie.Domain = m.Config.Domain
}

http.SetCookie(w, cookie)
}

// run decorated handler
Expand Down
3 changes: 3 additions & 0 deletions internal/server/auth/method/oidc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package oidc
import (
"context"
"fmt"
"strings"
"time"

"github.com/coreos/go-oidc/v3/oidc"
Expand Down Expand Up @@ -158,6 +159,8 @@ func (s *Server) Callback(ctx context.Context, req *auth.CallbackRequest) (_ *au
}

func callbackURL(host, provider string) string {
// strip trailing slash from host
host = strings.TrimSuffix(host, "/")
return host + "/auth/v1/method/oidc/" + provider + "/callback"
}

Expand Down
42 changes: 42 additions & 0 deletions internal/server/auth/method/oidc/server_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package oidc

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestCallbackURL(t *testing.T) {
tests := []struct {
name string
host string
want string
}{
{
name: "no trailing slash",
host: "localhost:8080",
want: "localhost:8080/auth/v1/method/oidc/foo/callback",
},
{
name: "with trailing slash",
host: "localhost:8080/",
want: "localhost:8080/auth/v1/method/oidc/foo/callback",
},
{
name: "with protocol",
host: "http://localhost:8080",
want: "http://localhost:8080/auth/v1/method/oidc/foo/callback",
},
{
name: "with protocol and trailing slash",
host: "http://localhost:8080/",
want: "http://localhost:8080/auth/v1/method/oidc/foo/callback",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := callbackURL(tt.host, "foo")
assert.Equal(t, tt.want, got)
})
}
}

0 comments on commit ea1b732

Please sign in to comment.