Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED] validation to return error when token are in the wrong context #149

Merged
merged 1 commit into from
Mar 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion activation_claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,14 @@ func (a *ActivationClaims) Payload() interface{} {

// Validate checks the claims
func (a *ActivationClaims) Validate(vr *ValidationResults) {
a.ClaimsData.Validate(vr)
a.validateWithTimeChecks(vr, true)
}

// Validate checks the claims
func (a *ActivationClaims) validateWithTimeChecks(vr *ValidationResults, timeChecks bool) {
if timeChecks {
a.ClaimsData.Validate(vr)
}
a.Activation.Validate(vr)
if a.IssuerAccount != "" && !nkeys.IsValidPublicAccountKey(a.IssuerAccount) {
vr.AddError("account_id is not an account public key")
Expand Down
35 changes: 21 additions & 14 deletions imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
}

if i.Account == "" {
vr.AddWarning("account to import from is not specified")
vr.AddError("account to import from is not specified")
}

i.Subject.Validate(vr)
Expand All @@ -78,46 +78,53 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {

if i.Token != "" {
// Check to see if its an embedded JWT or a URL.
if url, err := url.Parse(i.Token); err == nil && url.Scheme != "" {
if u, err := url.Parse(i.Token); err == nil && u.Scheme != "" {
c := &http.Client{Timeout: 5 * time.Second}
resp, err := c.Get(url.String())
resp, err := c.Get(u.String())
if err != nil {
vr.AddWarning("import %s contains an unreachable token URL %q", i.Subject, i.Token)
vr.AddError("import %s contains an unreachable token URL %q", i.Subject, i.Token)
}

if resp != nil {
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
vr.AddWarning("import %s contains an unreadable token URL %q", i.Subject, i.Token)
vr.AddError("import %s contains an unreadable token URL %q", i.Subject, i.Token)
} else {
act, err = DecodeActivationClaims(string(body))
if err != nil {
vr.AddWarning("import %s contains a url %q with an invalid activation token", i.Subject, i.Token)
vr.AddError("import %s contains a URL %q with an invalid activation token", i.Subject, i.Token)
}
}
}
} else {
var err error
act, err = DecodeActivationClaims(i.Token)
if err != nil {
vr.AddWarning("import %q contains an invalid activation token", i.Subject)
vr.AddError("import %q contains an invalid activation token", i.Subject)
}
}
}

if act != nil {
if act.Issuer != i.Account {
vr.AddWarning("activation token doesn't match account for import %q", i.Subject)
if !(act.Issuer == i.Account || act.IssuerAccount == i.Account) {
vr.AddError("activation token doesn't match account for import %q", i.Subject)
}

if act.ClaimsData.Subject != actPubKey {
vr.AddWarning("activation token doesn't match account it is being included in, %q", i.Subject)
vr.AddError("activation token doesn't match account it is being included in, %q", i.Subject)
}
if act.ImportType != i.Type {
vr.AddError("mismatch between token import type %s and type of import %s", act.ImportType, i.Type)
}
act.validateWithTimeChecks(vr, false)
subj := i.Subject
if i.IsService() && i.To != "" {
subj = i.To
}
if !subj.IsContainedIn(act.ImportSubject) {
vr.AddError("activation token import subject %q doesn't match import %q", act.ImportSubject, i.Subject)
}
} else {
vr.AddWarning("no activation provided for import %s", i.Subject)
}

}

// Imports is a list of import structs
Expand Down
69 changes: 22 additions & 47 deletions imports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,19 @@ func TestImportValidation(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
if !vr.IsEmpty() {
t.Errorf("imports should not generate an issue")
}

i.Type = Service
vr = CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
if !vr.IsEmpty() {
t.Errorf("imports should not generate an issue")
}

activation := NewActivationClaims(akp)
activation.Max = 1024 * 1024
activation.Expires = time.Now().Add(time.Duration(time.Hour)).UTC().Unix()
activation.Expires = time.Now().Add(time.Hour).UTC().Unix()

activation.ImportSubject = "test"
activation.ImportType = Stream
Expand Down Expand Up @@ -96,19 +86,15 @@ func TestInvalidImportToken(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports with a bad token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("invalid type shouldnt be blocking")
if !vr.IsBlocking(true) {
t.Errorf("bad token should be blocking")
}
}

func TestInvalidImportURL(t *testing.T) {
ak := createAccountNKey(t)
akp := publicKey(ak, t)
i := &Import{Subject: "foo", Account: akp, Token: "foo://bad token url", To: "bar", Type: Stream}
i := &Import{Subject: "foo", Account: akp, Token: "foo://bad-token-url", To: "bar", Type: Stream}

vr := CreateValidationResults()
i.Validate("", vr)
Expand All @@ -117,8 +103,8 @@ func TestInvalidImportURL(t *testing.T) {
t.Errorf("imports with a bad token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("invalid type shouldnt be blocking")
if !vr.IsBlocking(true) {
t.Errorf("invalid type should be blocking")
}
}

Expand All @@ -127,15 +113,11 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
ak2 := createAccountNKey(t)
akp := publicKey(ak, t)
akp2 := publicKey(ak2, t)
i := &Import{Subject: "test", Account: akp2, To: "bar", Type: Stream}
i := &Import{Subject: "bar", Account: akp2, To: "test", Type: Service}

vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
}
Expand All @@ -144,10 +126,6 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
vr = CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
}
Expand All @@ -157,7 +135,7 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
activation.Expires = time.Now().Add(time.Duration(time.Hour)).UTC().Unix()

activation.ImportSubject = "test"
activation.ImportType = Stream
activation.ImportType = Service
actJWT := encode(activation, ak2, t)

i.Token = actJWT
Expand Down Expand Up @@ -187,18 +165,19 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
t.Errorf("imports with wrong issuer")
}
}

func TestMissingAccountInImport(t *testing.T) {
i := &Import{Subject: "foo", To: "bar", Type: Stream}

vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 2 {
t.Errorf("imports without token or url should warn the caller, as should missing account")
if len(vr.Issues) != 1 {
t.Errorf("expected only one issue")
}

if vr.IsBlocking(true) {
t.Errorf("Missing Account is not blocking, must import failures are warnings")
if !vr.IsBlocking(true) {
t.Errorf("Missing Account is blocking")
}
}

Expand All @@ -210,10 +189,6 @@ func TestServiceImportWithWildcard(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 2 {
t.Errorf("imports without token or url should warn the caller, as should wildcard service")
}

if !vr.IsBlocking(true) {
t.Errorf("expected service import with a wildcard subject to be a blocking error")
}
Expand All @@ -225,8 +200,8 @@ func TestStreamImportWithWildcardPrefix(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 3 {
t.Errorf("should have registered 3 issues with this import, got %d", len(vr.Issues))
if len(vr.Issues) != 2 {
t.Errorf("should have registered 2 issues with this import, got %d", len(vr.Issues))
}

if !vr.IsBlocking(true) {
Expand All @@ -246,8 +221,8 @@ func TestImportsValidation(t *testing.T) {
vr := CreateValidationResults()
imports.Validate("", vr)

if len(vr.Issues) != 3 {
t.Errorf("imports without token or url should warn the caller x2, wildcard service as well")
if len(vr.Issues) != 1 {
t.Errorf("warn about wildcard service")
}

if !vr.IsBlocking(true) {
Expand Down Expand Up @@ -349,7 +324,7 @@ func TestImportSubjectValidation(t *testing.T) {
vr = CreateValidationResults()
i.Validate(akp, vr)

if !vr.IsEmpty() {
if vr.IsEmpty() {
t.Errorf("imports with non-contains subject should be not valid")
}

Expand Down
2 changes: 1 addition & 1 deletion v2/account_claims_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestNewAccountClaims(t *testing.T) {
account.InfoURL = "http://localhost/my-account/doc"
account.Description = "my account"
account.Imports = Imports{}
account.Imports.Add(&Import{Subject: "test", Name: "test import", Account: apk2, Token: actJWT, To: "my", Type: Stream})
account.Imports.Add(&Import{Subject: "test", Name: "test import", Account: apk2, Token: actJWT, LocalSubject: "my", Type: Stream})

vr := CreateValidationResults()
account.Validate(vr)
Expand Down
19 changes: 16 additions & 3 deletions v2/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
}

if i.Account == "" {
vr.AddWarning("account to import from is not specified")
vr.AddError("account to import from is not specified")
}

if i.GetTo() != "" {
vr.AddWarning("the field to has been deprecated (use LocalSubject instead)")
}

i.Subject.Validate(vr)
Expand All @@ -88,19 +92,28 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
var err error
act, err = DecodeActivationClaims(i.Token)
if err != nil {
vr.AddWarning("import %q contains an invalid activation token", i.Subject)
vr.AddError("import %q contains an invalid activation token", i.Subject)
}
}

if act != nil {
if !(act.Issuer == i.Account || act.IssuerAccount == i.Account) {
vr.AddError("activation token doesn't match account for import %q", i.Subject)
}

if act.ClaimsData.Subject != actPubKey {
vr.AddError("activation token doesn't match account it is being included in, %q", i.Subject)
}
if act.ImportType != i.Type {
vr.AddError("mismatch between token import type %s and type of import %s", act.ImportType, i.Type)
}
act.validateWithTimeChecks(vr, false)
subj := i.Subject
if i.IsService() && i.To != "" {
subj = i.To
}
if !subj.IsContainedIn(act.ImportSubject) {
vr.AddError("activation token import subject %q doesn't match import %q", act.ImportSubject, i.Subject)
}
}
}

Expand Down
Loading