Skip to content

Commit

Permalink
Minor improvements to the HTTP authentication middleware change
Browse files Browse the repository at this point in the history
- In our case a shallow copy of the HTTP request using .Context()
  instead of .Clone() is sufficient.
- Do a better job at explaining how the cookie encryption key is
  computed.
- Leave a 'reserved' marker in place for the 'listen_address' field, to
  make it easier for people to figure out how to migrate.
  • Loading branch information
EdSchouten committed Aug 29, 2023
1 parent e507452 commit 34e3e8d
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 15 deletions.
2 changes: 1 addition & 1 deletion pkg/http/authenticating_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ func (h *authenticatingHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
// not write a response to the client (e.g., emit a
// redirect). Forward the request to the underlying
// handler.
h.handler.ServeHTTP(w, r.Clone(auth.NewContextWithAuthenticationMetadata(r.Context(), metadata)))
h.handler.ServeHTTP(w, r.WithContext(auth.NewContextWithAuthenticationMetadata(r.Context(), metadata)))
}
}
6 changes: 5 additions & 1 deletion pkg/http/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func NewAuthenticatorFromConfiguration(policy *configuration.AuthenticationPolic
case *configuration.AuthenticationPolicy_Oidc:
// Select a name and encryption key for the session
// state cookie. Even though the configuration has a
// dedicted cookie seed field, we include the rest of
// dedicated cookie seed field, we include the rest of
// the configuration message as well. This ensures that
// any changes to the configuration automatically
// invalidate existing sessions.
Expand All @@ -74,6 +74,10 @@ func NewAuthenticatorFromConfiguration(policy *configuration.AuthenticationPolic
return nil, status.Error(codes.InvalidArgument, "Failed to marshal configuration to compute OIDC cookie seed")
}
cookieSeedHash := sha256.Sum256(fullCookieSeed)

// Let the first 128 bits of the seed hash be the name
// of the cookie, while the last 128 bits are used as
// the AES key for encrypting/signing the cookie value.
cookieName := base64.RawURLEncoding.EncodeToString(cookieSeedHash[:sha256.Size/2])
cookieCipher, err := aes.NewCipher(cookieSeedHash[sha256.Size/2:])
if err != nil {
Expand Down
11 changes: 10 additions & 1 deletion pkg/http/oidc_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,16 @@ type oidcAuthenticator struct {
// requests are authorized by an OAuth2 server. Authentication metadata
// is constructed by obtaining claims through the OpenID Connect user
// info endpoint, and transforming it using a JMESPath expression.
func NewOIDCAuthenticator(oauth2Config *oauth2.Config, userInfoURL string, metadataExtractor *jmespath.JMESPath, httpClient *http.Client, randomNumberGenerator random.ThreadSafeGenerator, cookieName string, cookieAEAD cipher.AEAD, clock clock.Clock) (Authenticator, error) {
func NewOIDCAuthenticator(
oauth2Config *oauth2.Config,
userInfoURL string,
metadataExtractor *jmespath.JMESPath,
httpClient *http.Client,
randomNumberGenerator random.ThreadSafeGenerator,
cookieName string,
cookieAEAD cipher.AEAD,
clock clock.Clock,
) (Authenticator, error) {
// Extract the path in the redirect URL of the OAuth2
// configuration, as we need to match it in incoming HTTP
// requests.
Expand Down
6 changes: 3 additions & 3 deletions pkg/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (
)

// NewServersFromConfigurationAndServe spawns HTTP servers as part of a
// program.Group, based on a configuration specification. The web
// servers are automatically terminated if the context associated with
// the group is canceled.
// program.Group, based on a configuration message. The web servers are
// automatically terminated if the context associated with the group is
// canceled.
func NewServersFromConfigurationAndServe(configurations []*configuration.ServerConfiguration, handler http.Handler, group program.Group) error {
for _, configuration := range configurations {
authenticator, err := NewAuthenticatorFromConfiguration(configuration.AuthenticationPolicy)
Expand Down
17 changes: 9 additions & 8 deletions pkg/proto/configuration/global/global.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion pkg/proto/configuration/global/global.proto
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,14 @@ message Configuration {
}

message DiagnosticsHTTPServerConfiguration {
// Was 'listen_address'. The listen address of the diagnostics HTTP
// server can now be configured through the 'http_servers' option.
reserved 1;

// Default endpoints:
// - /-/healthy: Returns HTTP 200 OK if the application managed to
// start successfully.
repeated buildbarn.configuration.http.ServerConfiguration http_servers = 1;
repeated buildbarn.configuration.http.ServerConfiguration http_servers = 5;

// Enables endpoints:
// - /debug/pprof/*: Endpoints for Go's pprof debug tool.
Expand Down
4 changes: 4 additions & 0 deletions pkg/proto/configuration/http/http.proto
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ message OIDCAuthenticationPolicy {
// service returns refresh tokens. This permits the HTTP server to
// continue sessions with fewer redirects to the authorization
// service.
//
// A list of commonly used scopes can be found in the OpenID Connect
// specification:
// https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims
repeated string scopes = 8;

// A random seed of how cookies containing session state are named,
Expand Down

0 comments on commit 34e3e8d

Please sign in to comment.