github-personal/certificates
1
0
Fork 0
mirror of https://github.com/outbackdingo/certificates.git synced 2026-01-27 10:18:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve token validation error messages and use net/url

Browse Source
This commit is contained in:
Herman Slatman
2024-03-06 15:16:23 +01:00
parent 64c5f19520
commit 6eb4662120
3 changed files with 17 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
acme/challenge.go
View File

@@ -641,19 +641,19 @@ func parseAndVerifyWireAccessToken(v wireVerifyParams) (*wireAccessToken, *wireD
}
if accessToken.Challenge == "" {
return nil, nil, errors.New("access token challenge must not be empty")
return nil, nil, errors.New("access token challenge 'chal' must not be empty")
}
if accessToken.Cnf.Kid == "" || accessToken.Cnf.Kid != v.dpopKeyID {
return nil, nil, fmt.Errorf("expected kid %q; got %q", v.dpopKeyID, accessToken.Cnf.Kid)
return nil, nil, fmt.Errorf("expected 'kid' %q; got %q", v.dpopKeyID, accessToken.Cnf.Kid)
}
if accessToken.ClientID != v.wireID.ClientID {
return nil, nil, fmt.Errorf("invalid Wire client ID %q", accessToken.ClientID)
return nil, nil, fmt.Errorf("invalid Wire 'client_id' %q", accessToken.ClientID)
}
if accessToken.Expiry.Time().After(v.t.Add(time.Hour)) {
return nil, nil, fmt.Errorf("'exp' %s is too far into the future", accessToken.Expiry.Time().String())
return nil, nil, fmt.Errorf("token expiry 'exp' %s is too far into the future", accessToken.Expiry.Time().String())
}
if accessToken.Scope != "wire_client_id" {
return nil, nil, fmt.Errorf("invalid Wire scope %q", accessToken.Scope)
return nil, nil, fmt.Errorf("invalid Wire 'scope' %q", accessToken.Scope)
}
dpopJWT, err := jose.ParseSigned(accessToken.Proof)
@@ -685,7 +685,7 @@ func parseAndVerifyWireAccessToken(v wireVerifyParams) (*wireAccessToken, *wireD
return nil, nil, fmt.Errorf("failed DPoP validation: %w", err)
}
if wireDpop.HTU == "" || wireDpop.HTU != v.issuer { // DPoP doesn't contains "iss" claim, but has it in the "htu" claim
return nil, nil, fmt.Errorf("DPoP contains invalid issuer (htu) %q", wireDpop.HTU)
return nil, nil, fmt.Errorf("DPoP contains invalid issuer 'htu' %q", wireDpop.HTU)
}
if wireDpop.Expiry.Time().After(v.t.Add(time.Hour)) {
return nil, nil, fmt.Errorf("'exp' %s is too far into the future", wireDpop.Expiry.Time().String())
@@ -694,10 +694,10 @@ func parseAndVerifyWireAccessToken(v wireVerifyParams) (*wireAccessToken, *wireD
return nil, nil, fmt.Errorf("DPoP contains invalid Wire client ID %q", wireDpop.ClientID)
}
if wireDpop.Nonce == "" || wireDpop.Nonce != accessToken.Nonce {
return nil, nil, fmt.Errorf("DPoP contains invalid nonce %q", wireDpop.Nonce)
return nil, nil, fmt.Errorf("DPoP contains invalid 'nonce' %q", wireDpop.Nonce)
}
if wireDpop.Challenge == "" || wireDpop.Challenge != accessToken.Challenge {
return nil, nil, fmt.Errorf("DPoP contains invalid challenge %q", wireDpop.Challenge)
return nil, nil, fmt.Errorf("DPoP contains invalid challenge 'chal' %q", wireDpop.Challenge)
}
// TODO(hs): can we use the wireDpopJwt and map that instead of doing Claims() twice?
@@ -708,26 +708,26 @@ func parseAndVerifyWireAccessToken(v wireVerifyParams) (*wireAccessToken, *wireD
challenge, ok := dpopToken["chal"].(string)
if !ok {
return nil, nil, fmt.Errorf("invalid challenge in Wire DPoP token")
return nil, nil, fmt.Errorf("invalid challenge 'chal' in Wire DPoP token")
}
if challenge == "" || challenge != v.chToken {
return nil, nil, fmt.Errorf("invalid Wire DPoP challenge %q", challenge)
return nil, nil, fmt.Errorf("invalid Wire DPoP challenge 'chal' %q", challenge)
}
handle, ok := dpopToken["handle"].(string)
if !ok {
return nil, nil, fmt.Errorf("invalid handle in Wire DPoP token")
return nil, nil, fmt.Errorf("invalid 'handle' in Wire DPoP token")
}
if handle == "" || handle != v.wireID.Handle {
return nil, nil, fmt.Errorf("invalid Wire client handle %q", handle)
return nil, nil, fmt.Errorf("invalid Wire client 'handle' %q", handle)
}
name, ok := dpopToken["name"].(string)
if !ok {
return nil, nil, fmt.Errorf("invalid display name in Wire DPoP token")
return nil, nil, fmt.Errorf("invalid display 'name' in Wire DPoP token")
}
if name == "" || name != v.wireID.Name {
return nil, nil, fmt.Errorf("invalid Wire client display name %q", name)
return nil, nil, fmt.Errorf("invalid Wire client display 'name' %q", name)
}
return &accessToken, &dpopToken, nil

5
acme/wire/id.go
View File

@@ -4,9 +4,8 @@ import (
"encoding/json"
"errors"
"fmt"
"net/url"
"strings"
"go.step.sm/crypto/kms/uri"
)
type UserID struct {
@@ -71,7 +70,7 @@ type ClientID struct {
//
// where '!' is used as a separator between the user id & device id.
func ParseClientID(clientID string) (ClientID, error) {
clientIDURI, err := uri.Parse(clientID)
clientIDURI, err := url.Parse(clientID)
if err != nil {
return ClientID{}, fmt.Errorf("invalid Wire client ID URI %q: %w", clientID, err)
}

2
acme/wire/id_test.go
View File

@@ -81,7 +81,7 @@ func TestParseClientID(t *testing.T) {
expectedErr error
}{
{name: "ok", clientID: "wireapp://CzbfFjDOQrenCbDxVmgnFw!594930e9d50bb175@wire.com", want: ClientID{Scheme: "wireapp", Username: "CzbfFjDOQrenCbDxVmgnFw", DeviceID: "594930e9d50bb175", Domain: "wire.com"}},
{name: "fail/uri", clientID: "bla", expectedErr: errors.New(`invalid Wire client ID URI "bla": error parsing bla: scheme is missing`)},
{name: "fail/uri", clientID: "bla", expectedErr: errors.New(`invalid Wire client ID scheme ""; expected "wireapp"`)},
{name: "fail/scheme", clientID: "not-wireapp://bla.com", expectedErr: errors.New(`invalid Wire client ID scheme "not-wireapp"; expected "wireapp"`)},
{name: "fail/username", clientID: "wireapp://user@wire.com", expectedErr: errors.New(`invalid Wire client ID username "user"`)},
}