Skip to content

Commit

Permalink
Switch to using a single JWT library and unify api package
Browse files Browse the repository at this point in the history
The current implementation uses two JWT packages: github.com/golang-jwt/jwt and github.com/lestrrat-go/jwx/v2/jwt. These packages have very different APIs and developer surfaces. This PR standardizes everything on github.com/lestrrat-go/jwx/v2/jwt since we need to the jwx package anyway for jwk parsing and jws signing.

Because github.com/lestrrat-go/jwx/v2/jwt uses an interface to store claims, this PR introduces new top-level functions for getting/setting justifications on a token. This is significantly less error-prone than relying on "justs" to be populated in the JSON and enforces a structure. The downside is that it requires an extra option during parsing, but that is abstracted away into helpers. These helpers are important because the previous implementation had some extremely rough edges because slices are passed by reference. This caused data races and extremely-difficult-to-debug side-effects. The helpers make a copy of slices to ensure this doesn't happen.

One of the major downsides of the github.com/lestrrat-go/jwx/v2/jwt package is that there is no way to create "unsigned" tokens. github.com/golang-jwt/jwt has a "SigningString()" function for this purpose, but that does not exist in the new package. In retrospect, I'm not convinced that using the ".NOT_SIGNED" suffix is the right solution for breakglass, and I think we should explore using HMAC-signed keys instead. That being said, I was able to preserve backwards-compatibility with the existing implementation by signing the key, stripping off the signature, and appending ".NOT_SIGNED". This is less than ideal, but I think any refactors to the breakglass semantics should be done in a separate PR and discussion.

This PR also has some general code cleanup, such as being consistent with the name of the jvsapis/v0 package import.
  • Loading branch information
sethvargo committed Sep 21, 2022
1 parent 051e121 commit d07c8fb
Show file tree
Hide file tree
Showing 19 changed files with 914 additions and 1,091 deletions.
107 changes: 107 additions & 0 deletions apis/v0/jwt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package v0

import (
"fmt"

"github.com/lestrrat-go/jwx/v2/jwt"
)

const (
// jwtJustificationsKey is the key in the JWT where justifications are stored.
// Ideally this would be "justifications", but the RFC and various online
// resources recommend key names be as short as possible to keep the JWTs
// small. Akamai recommends less than 8 characters and Okta recommends less
// than 6.
jwtJustificationsKey string = "justs"
)

// WithTypedJustifications is an option for parsing JWTs that will convert
// decode the [Justification] claims into the correct Go structure. If this is
// not supplied, the claims will be "any" and future type assertions will fail.
func WithTypedJustifications() jwt.ParseOption {
return jwt.WithTypedClaim(jwtJustificationsKey, []*Justification{})
}

// GetJustifications retrieves a copy of the justifications on the token. If the
// token does not have any justifications, it returns an empty slice of
// justifications. Modifying the slice does not modify the underlying token -
// you must call SetJustifications to update the data on the token.
func GetJustifications(t jwt.Token) ([]*Justification, error) {
if t == nil {
return nil, fmt.Errorf("token cannot be nil")
}

raw, ok := t.Get(jwtJustificationsKey)
if !ok {
return []*Justification{}, nil
}

typ, ok := raw.([]*Justification)
if !ok {
return nil, fmt.Errorf("found justifications, but was %T (expected %T)",
raw, []*Justification{})
}

// Make a copy of the slice so we don't modify the underlying data structure.
cp := make([]*Justification, 0, len(typ))
cp = append(cp, typ...)
return cp, nil
}

// SetJustifications updates the justifications on the token. It overwrites any
// existing values and uses a copy of the inbound slice.
func SetJustifications(t jwt.Token, justifications []*Justification) error {
if t == nil {
return fmt.Errorf("token cannot be nil")
}

cp := make([]*Justification, 0, len(justifications))
cp = append(cp, justifications...)
return t.Set(jwtJustificationsKey, cp)
}

// AppendJustification appends the given justification to the end of the current
// justifications list. It does not check for duplication and does not lock the
// token. There is a possible race between when the claims are read and when the
// claims are set back on the token. Callers should use [SetJustifications]
// directly to avoid this race.
func AppendJustification(t jwt.Token, justification *Justification) error {
if t == nil {
return fmt.Errorf("token cannot be nil")
}

justifications, err := GetJustifications(t)
if err != nil {
return fmt.Errorf("failed to get justifications: %w", err)
}

justifications = append(justifications, justification)
if err := SetJustifications(t, justifications); err != nil {
return fmt.Errorf("failed to set justifications: %w", err)
}
return nil
}

// ClearJustifications removes the justifications from the token by deleting the
// entire key.
func ClearJustifications(t jwt.Token) error {
if t == nil {
return fmt.Errorf("token cannot be nil")
}

return t.Remove(jwtJustificationsKey)
}
26 changes: 0 additions & 26 deletions apis/v0/jwt_specification.go

This file was deleted.

Loading

0 comments on commit d07c8fb

Please sign in to comment.