Skip to content

Commit

Permalink
Merge pull request #3 from uc-cdis/fix/aud-scopes
Browse files Browse the repository at this point in the history
(PXP-6617): remove scopes from jwt aud claim
  • Loading branch information
vpsx authored May 26, 2021
2 parents 17b5f35 + 43330b1 commit 909ef98
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 42 deletions.
10 changes: 7 additions & 3 deletions authutils/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@ func expired(timestamp int64) error {
return errors.New(msg)
}

func missingAudience(missingAud string, containsAuds []string) error {
containsString := strings.Join(containsAuds, ", ")
msg := fmt.Sprintf("token missing required audience: %s; contains: %s\n", missingAud, containsString)
func missingScope(missingScope string, containsScopes []string) error {
containsString := strings.Join(containsScopes, ", ")
msg := fmt.Sprintf("token missing required scope: %s; contains: %s\n", missingScope, containsString)
return errors.New(msg)
}

func missingKey(keyID string) error {
return fmt.Errorf("no key exists with ID: %s", keyID)
}
5 changes: 2 additions & 3 deletions authutils/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package authutils

import (
"encoding/json"
"errors"
"fmt"
"net/http"

Expand Down Expand Up @@ -41,7 +40,7 @@ func (manager *KeysManager) Lookup(keyID string) (*jose.JSONWebKey, error) {
jwk, exists = manager.KeyMap[keyID]
// If still no key is found, return an error.
if !exists {
return jwk, errors.New(fmt.Sprintf("no key exists with ID: %s", keyID))
return nil, missingKey(keyID)
}
}
return jwk, nil
Expand Down Expand Up @@ -71,7 +70,7 @@ func (manager *KeysManager) Refresh() error {
// Get the JSON response from the URL configured in the manager.
resp, err := http.Get(manager.URL)
if err != nil {
return err
return fmt.Errorf("couldn't get keys from %s: %s", manager.URL, err.Error())
}

// Parse the response JSON into a jose.JSONWebKeySet.
Expand Down
14 changes: 8 additions & 6 deletions authutils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,23 @@ func publicKeyToJWK(keyID string, publicKey *rsa.PublicKey) jose.JSONWebKey {
func makeDefaultClaims() Claims {
exp := int(time.Now().Unix() + 1000)
exampleClaims := Claims{
"aud": []string{"test"},
"iss": "https://example-iss.net",
"exp": exp,
"pur": "access",
"scope": []string{"test"},
"iss": "https://example-iss.net",
"exp": exp,
"pur": "access",
}

return exampleClaims
}

func makeDefaultExpected() Expected {
purpose := "access"
now := time.Now().Unix()
exp := &now
expected := Expected{
Audiences: []string{"test"},
Scopes: []string{"test"},
Issuers: []string{"https://example-iss.net"},
Expiration: time.Now().Unix(),
Expiration: exp,
Purpose: &purpose,
}
return expected
Expand Down
64 changes: 35 additions & 29 deletions authutils/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ func checkExpiration(claims *Claims, now int64) error {

// checkIssuer validates the `iss` field in the claims.
func checkIssuer(claims *Claims, allowed []string) error {
if allowed == nil {
return nil
}
tokenIss, exists := (*claims)["iss"]
if !exists {
return missingField("iss")
Expand All @@ -105,30 +108,34 @@ func checkIssuer(claims *Claims, allowed []string) error {
return nil
}

// checkAudience validates the `aud` field in the claims.
func checkAudience(claims *Claims, expected []string) error {
tokenAud, exists := (*claims)["aud"]
// checkScope validates the `scope` field in the claims.
func checkScope(claims *Claims, expected []string) error {
// if token has a scope field but no scopes are expected this is fine
if len(expected) == 0 {
return nil
}
tokenScope, exists := (*claims)["scope"]
if !exists {
return missingField("aud")
return missingField("scope")
}
var aud []string
switch a := tokenAud.(type) {
var scope []string
switch a := tokenScope.(type) {
case []string:
aud = a
scope = a
case []interface{}:
for _, value := range a {
valueString, casted := value.(string)
if !casted {
return fieldTypeError("aud", tokenAud, "[]string")
return fieldTypeError("scope", tokenScope, "[]string")
}
aud = append(aud, valueString)
scope = append(scope, valueString)
}
default:
return fieldTypeError("aud", tokenAud, "[]string")
return fieldTypeError("scope", tokenScope, "[]string")
}
for _, expectedAud := range expected {
if !contains(expectedAud, aud) {
return missingAudience(expectedAud, aud)
for _, expectedScope := range expected {
if !contains(expectedScope, scope) {
return missingScope(expectedScope, scope)
}
}
return nil
Expand All @@ -154,26 +161,21 @@ func checkPurpose(claims *Claims, expected *string) error {
// Expected represents some values which are used to validate the claims in a
// token.
type Expected struct {
// Audiences is a list of expected receivers or uses of the token.
Audiences []string
// Scopes is a list of expected uses of the token.
Scopes []string `json:"scope"`
// Expiration is the Unix timestamp at which the token becomes expired.
Expiration int64
Expiration *int64 `json:"exp"`
// Issuers is a list of acceptable issuers to expect tokens to contain.
Issuers []string
Issuers []string `json:"iss"`
// Purpose is an optional field indicating the type of the token (access,
// refresh, etc.)
Purpose *string
Purpose *string `json:"pur"`
}

// selfValidate ensures that the fields provided in Expected are valid. For
// example, to validate some Claims the validator must identify with at least
// one audience (`aud`) in the claims, so these may not be empty.
// See https://tools.ietf.org/html/rfc7519 for general information on JWTs and
// basic validation, and see https://tools.ietf.org/html/rfc7523 for
// considerations for validation specific to using JWTs for the OAuth2 flow.
func (expected *Expected) selfValidate() error {
// Must expect at least one audience.
if len(expected.Audiences) == 0 {
return validationError("must validate against at least one audience")
}

if expected.Purpose != nil {
// Must expect one of these given purposes.
if !contains(*expected.Purpose, ALLOWED_PURPOSES) {
Expand All @@ -194,14 +196,18 @@ func (expected *Expected) Validate(claims *Claims) error {
return err
}

now := time.Now().Unix()
if err := checkExpiration(claims, now); err != nil {
exp := expected.Expiration
if exp == nil {
now := time.Now().Unix()
exp = &now
}
if err := checkExpiration(claims, *exp); err != nil {
return err
}
if err := checkIssuer(claims, expected.Issuers); err != nil {
return err
}
if err := checkAudience(claims, expected.Audiences); err != nil {
if err := checkScope(claims, expected.Scopes); err != nil {
return err
}
if err := checkPurpose(claims, expected.Purpose); err != nil {
Expand Down
1 change: 0 additions & 1 deletion authutils/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
// REQUIRED_CLAIMS lists the claims which absolutely must appear in a token,
// whose absence will cause it not to validate.
var REQUIRED_CLAIMS []string = []string{
"aud",
"exp",
"iss",
}
Expand Down

0 comments on commit 909ef98

Please sign in to comment.