Skip to content

Commit

Permalink
Adds a check for users re-submitting the redacted token.
Browse files Browse the repository at this point in the history
  • Loading branch information
James Phillips committed Feb 25, 2016
1 parent 483898a commit 7d39211
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
8 changes: 7 additions & 1 deletion consul/prepared_query_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,13 @@ func parseQuery(query *structs.PreparedQuery) error {
// transaction. Otherwise, people could "steal" queries that they don't
// have proper ACL rights to change.
// - Session is optional and checked for integrity during the transaction.
// - Token is checked when a query is executed.

// Token is checked when the query is executed, but we do make sure the
// user hasn't accidentally pasted-in the special redacted token name,
// which if we allowed in would be super hard to debug and understand.
if query.Token == redactedToken {
return fmt.Errorf("Bad Token '%s', it looks like a query definition with a redacted token was submitted", query.Token)
}

// Parse the service query sub-structure.
if err := parseService(&query.Service); err != nil {
Expand Down
11 changes: 11 additions & 0 deletions consul/prepared_query_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,17 @@ func TestPreparedQuery_parseQuery(t *testing.T) {
t.Fatalf("err: %v", err)
}

query.Token = redactedToken
err = parseQuery(query)
if err == nil || !strings.Contains(err.Error(), "Bad Token") {
t.Fatalf("bad: %v", err)
}

query.Token = "adf4238a-882b-9ddc-4a9d-5b6758e4159e"
if err := parseQuery(query); err != nil {
t.Fatalf("err: %v", err)
}

query.Service.Failover.NearestN = -1
err = parseQuery(query)
if err == nil || !strings.Contains(err.Error(), "Bad NearestN") {
Expand Down
8 changes: 4 additions & 4 deletions website/source/docs/agent/http/query.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ queries and all consistency modes.

If ACLs are enabled, then the client will only see prepared queries for which their
token has `query` read privileges. A management token will be able to see all
prepared queries. Tokens will be displayed as `<hidden>` unless a management token is
used.
prepared queries. Tokens will be redacted and displayed as `<hidden>` unless a
management token is used.

This returns a JSON list of prepared queries, which looks like:

Expand Down Expand Up @@ -233,8 +233,8 @@ status code will be returned.

If ACLs are enabled, then the client will only see prepared queries for which their
token has `query` read privileges. A management token will be able to see all
prepared queries. Tokens will be displayed as `<hidden>` unless a management token is
used.
prepared queries. Tokens will be redacted and displayed as `<hidden>` unless a
management token is used.

#### DELETE Method

Expand Down

0 comments on commit 7d39211

Please sign in to comment.