Skip to content

Commit 18eecaf

Browse files
author
Eduardo Gomes
committed
AUTH-6633 Fix cloudflared access login + warp as auth
## Summary cloudflared access login and cloudflared access curl fails when the Access application has warp_as_auth enabled. This bug originates from a 4 year old inconsistency where tokens signed by the nginx-fl-access module include 'aud' as a string, while tokens signed by the access authentication worker include 'aud' as an array of strings. When the new(ish) feature warp_as_auth is enabled for the app, the fl module signs the token as opposed to the worker like usually. I'm going to bring this up to the Access team, and try to figure out a way to consolidate this discrepancy without breaking behaviour. Meanwhile we have this [CUSTESC ](https://jira.cfdata.org/browse/CUSTESC-47987), so I'm making cloudflared more lenient by accepting both []string and string in the token 'aud' field. Tested this by compiling and running cloudflared access curls to my domains Closes AUTH-6633
1 parent 4eb0f8c commit 18eecaf

File tree

2 files changed

+89
-8
lines changed

2 files changed

+89
-8
lines changed

token/token.go

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type signalHandler struct {
5353
}
5454

5555
type jwtPayload struct {
56-
Aud []string `json:"aud"`
56+
Aud []string `json:"-"`
5757
Email string `json:"email"`
5858
Exp int `json:"exp"`
5959
Iat int `json:"iat"`
@@ -68,6 +68,34 @@ type transferServiceResponse struct {
6868
OrgToken string `json:"org_token"`
6969
}
7070

71+
func (p *jwtPayload) UnmarshalJSON(data []byte) error {
72+
type Alias jwtPayload
73+
if err := json.Unmarshal(data, (*Alias)(p)); err != nil {
74+
return err
75+
}
76+
var audParser struct {
77+
Aud any `json:"aud"`
78+
}
79+
if err := json.Unmarshal(data, &audParser); err != nil {
80+
return err
81+
}
82+
switch aud := audParser.Aud.(type) {
83+
case string:
84+
p.Aud = []string{aud}
85+
case []any:
86+
for _, a := range aud {
87+
s, ok := a.(string)
88+
if !ok {
89+
return errors.New("aud array contains non-string elements")
90+
}
91+
p.Aud = append(p.Aud, s)
92+
}
93+
default:
94+
return errors.New("aud field is not a string or an array of strings")
95+
}
96+
return nil
97+
}
98+
7199
func (p jwtPayload) isExpired() bool {
72100
return int(time.Now().Unix()) > p.Exp
73101
}
@@ -182,7 +210,9 @@ func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, log *zerolog.
182210
if err = fileLockAppToken.Acquire(); err != nil {
183211
return "", errors.Wrap(err, "failed to acquire app token lock")
184212
}
185-
defer fileLockAppToken.Release()
213+
defer func() {
214+
_ = fileLockAppToken.Release()
215+
}()
186216

187217
// check to see if another process has gotten a token while we waited for the lock
188218
if token, err := GetAppTokenIfExists(appInfo); token != "" && err == nil {
@@ -202,7 +232,9 @@ func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, log *zerolog.
202232
if err = fileLockOrgToken.Acquire(); err != nil {
203233
return "", errors.Wrap(err, "failed to acquire org token lock")
204234
}
205-
defer fileLockOrgToken.Release()
235+
defer func() {
236+
_ = fileLockOrgToken.Release()
237+
}()
206238
// check if an org token has been created since the lock was acquired
207239
orgToken, err = GetOrgTokenIfExists(appInfo.AuthDomain)
208240
}
@@ -218,7 +250,6 @@ func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, log *zerolog.
218250
}
219251
}
220252
return getTokensFromEdge(appURL, appInfo.AppAUD, appTokenPath, orgTokenPath, useHostOnly, log)
221-
222253
}
223254

224255
// getTokensFromEdge will attempt to use the transfer service to retrieve an app and org token, save them to disk,
@@ -250,7 +281,6 @@ func getTokensFromEdge(appURL *url.URL, appAUD, appTokenPath, orgTokenPath strin
250281
}
251282

252283
return resp.AppToken, nil
253-
254284
}
255285

256286
// GetAppInfo makes a request to the appURL and stops at the first redirect. The 302 location header will contain the
@@ -320,7 +350,6 @@ func handleRedirects(req *http.Request, via []*http.Request, orgToken string) er
320350
}
321351
}
322352
}
323-
324353
}
325354

326355
// stop after hitting authorized endpoint since it will contain the app token
@@ -408,7 +437,6 @@ func GetAppTokenIfExists(appInfo *AppInfo) (string, error) {
408437
return "", err
409438
}
410439
return token.CompactSerialize()
411-
412440
}
413441

414442
// GetTokenIfExists will return the token from local storage if it exists and not expired

token/token_test.go

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package token
22

33
import (
4+
"encoding/json"
45
"net/http"
56
"net/url"
67
"testing"
@@ -11,7 +12,7 @@ func TestHandleRedirects_AttachOrgToken(t *testing.T) {
1112
via := []*http.Request{}
1213
orgToken := "orgTokenValue"
1314

14-
handleRedirects(req, via, orgToken)
15+
_ = handleRedirects(req, via, orgToken)
1516

1617
// Check if the orgToken cookie is attached
1718
cookies := req.Cookies()
@@ -80,3 +81,55 @@ func TestHandleRedirects_StopAtAuthorizedEndpoint(t *testing.T) {
8081
t.Errorf("Expected ErrUseLastResponse, got %v", err)
8182
}
8283
}
84+
85+
func TestJwtPayloadUnmarshal_AudAsString(t *testing.T) {
86+
jwt := `{"aud":"7afbdaf987054f889b3bdd0d29ebfcd2"}`
87+
var payload jwtPayload
88+
if err := json.Unmarshal([]byte(jwt), &payload); err != nil {
89+
t.Errorf("Expected no error, got %v", err)
90+
}
91+
if len(payload.Aud) != 1 || payload.Aud[0] != "7afbdaf987054f889b3bdd0d29ebfcd2" {
92+
t.Errorf("Expected aud to be 7afbdaf987054f889b3bdd0d29ebfcd2, got %v", payload.Aud)
93+
}
94+
}
95+
96+
func TestJwtPayloadUnmarshal_AudAsSlice(t *testing.T) {
97+
jwt := `{"aud":["7afbdaf987054f889b3bdd0d29ebfcd2", "f835c0016f894768976c01e076844efe"]}`
98+
var payload jwtPayload
99+
if err := json.Unmarshal([]byte(jwt), &payload); err != nil {
100+
t.Errorf("Expected no error, got %v", err)
101+
}
102+
if len(payload.Aud) != 2 || payload.Aud[0] != "7afbdaf987054f889b3bdd0d29ebfcd2" || payload.Aud[1] != "f835c0016f894768976c01e076844efe" {
103+
t.Errorf("Expected aud to be [7afbdaf987054f889b3bdd0d29ebfcd2, f835c0016f894768976c01e076844efe], got %v", payload.Aud)
104+
}
105+
}
106+
107+
func TestJwtPayloadUnmarshal_FailsWhenAudIsInt(t *testing.T) {
108+
jwt := `{"aud":123}`
109+
var payload jwtPayload
110+
err := json.Unmarshal([]byte(jwt), &payload)
111+
wantErr := "aud field is not a string or an array of strings"
112+
if err.Error() != wantErr {
113+
t.Errorf("Expected %v, got %v", wantErr, err)
114+
}
115+
}
116+
117+
func TestJwtPayloadUnmarshal_FailsWhenAudIsArrayOfInts(t *testing.T) {
118+
jwt := `{"aud": [999, 123] }`
119+
var payload jwtPayload
120+
err := json.Unmarshal([]byte(jwt), &payload)
121+
wantErr := "aud array contains non-string elements"
122+
if err.Error() != wantErr {
123+
t.Errorf("Expected %v, got %v", wantErr, err)
124+
}
125+
}
126+
127+
func TestJwtPayloadUnmarshal_FailsWhenAudIsOmitted(t *testing.T) {
128+
jwt := `{}`
129+
var payload jwtPayload
130+
err := json.Unmarshal([]byte(jwt), &payload)
131+
wantErr := "aud field is not a string or an array of strings"
132+
if err.Error() != wantErr {
133+
t.Errorf("Expected %v, got %v", wantErr, err)
134+
}
135+
}

0 commit comments

Comments
 (0)