Skip to content

Commit 9380678

Browse files
authored
Merge pull request #24 from devtron-labs/split-token-issue
feat: added reconstructing split token logic
2 parents fffb492 + 6eb2f56 commit 9380678

3 files changed

Lines changed: 96 additions & 10 deletions

File tree

middleware/AuthMiddleware.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"context"
2222
"encoding/json"
2323
"fmt"
24+
"github.com/devtron-labs/authenticator/oidc"
2425
log "github.com/sirupsen/logrus"
2526
"net/http"
2627
"strings"
@@ -40,17 +41,17 @@ func Authorizer(sessionManager *SessionManager, whitelistChecker func(url string
4041
// for external ci webhook request, will be authorize by api-token
4142
token = apiToken
4243
} else {
43-
cookie, _ := r.Cookie(argocdTokenHeaderKey)
44-
if cookie != nil {
45-
token = cookie.Value
44+
authCookieToken, _ := oidc.JoinCookies(oidc.AuthCookieName, r.Cookies())
45+
if authCookieToken != "" {
46+
token = authCookieToken
4647
r.Header.Set(tokenHeaderKey, token)
4748
}
48-
if token == "" && cookie == nil {
49+
if token == "" && authCookieToken == "" {
4950
token = r.Header.Get(tokenHeaderKey)
5051
}
5152
}
5253

53-
//users = append(users, "anonymous")
54+
// users = append(users, "anonymous")
5455
authEnabled := true
5556
pass := false
5657
config := GetConfig()
@@ -81,7 +82,7 @@ func Authorizer(sessionManager *SessionManager, whitelistChecker func(url string
8182
return
8283
}
8384

84-
//setting user id in context
85+
// setting user id in context
8586
context.WithValue(r.Context(), "userId", userId)
8687
}
8788
}

middleware/AuthMiddleware_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package middleware
2+
3+
import (
4+
"fmt"
5+
"github.com/devtron-labs/authenticator/oidc"
6+
"github.com/golang-jwt/jwt/v4"
7+
"log"
8+
"net/http"
9+
"strings"
10+
"testing"
11+
"time"
12+
)
13+
14+
type TestClaims struct {
15+
Groups map[string]string `json:"groups"`
16+
jwt.RegisteredClaims
17+
}
18+
19+
func Test_ReconstructSplitToken(t *testing.T) {
20+
verifySplitAndJoinToken := func(token string, tt *testing.T) {
21+
log.Print("token len : ", len(token))
22+
cookies, err := oidc.MakeCookieMetadata(oidc.AuthCookieName, token)
23+
if err != nil {
24+
t.Error(err)
25+
}
26+
r, _ := http.NewRequest("GET", "https://devtron.ai", nil)
27+
for _, c := range cookies {
28+
keyVal := strings.Split(c, "=")
29+
cookie := &http.Cookie{
30+
Name: keyVal[0],
31+
Value: keyVal[1],
32+
}
33+
r.AddCookie(cookie)
34+
}
35+
finalToken, err := oidc.JoinCookies(oidc.AuthCookieName, r.Cookies())
36+
if err != nil {
37+
t.Error(err)
38+
}
39+
if token != finalToken {
40+
tt.Fail()
41+
}
42+
43+
}
44+
45+
generateJWTToken := func(numClaims int) string {
46+
mapClaims := make(map[string]string)
47+
secret := []byte("T0p53cr3t")
48+
for i := 0; i < numClaims; i++ {
49+
mapClaims[fmt.Sprintf("key-%d", i)] = time.Now().String()
50+
}
51+
unsignedToken := jwt.NewWithClaims(jwt.SigningMethodHS256, TestClaims{Groups: mapClaims,
52+
RegisteredClaims: jwt.RegisteredClaims{
53+
Issuer: ApiTokenClaimIssuer,
54+
}})
55+
token, _ := unsignedToken.SignedString(secret)
56+
return token
57+
}
58+
59+
t.Run(">5kb token", func(tt *testing.T) {
60+
token := "eyJhbGciOiJSUzI1NiIsImtpZCI6Ijk2NDlhYzxxxyyyzzziOTNiNjBjMGRiMWVlN2Y1MmQ1ODZjOTMyNzAifQ.eyxxxyyyzzzodHRwczovL2RldnRyb24uazguZGVzYXJyb2xsby5lbXQuZXMvb3JjaGVzdHJhdG9yL2FwaS9kZXgiLCJzdWIiOiJDaVJrTjJVMk1tUTBZeTFpWmpObExUUTVaR010T1dNeFppMW1ZVFF3T0RZNE16WmpNeklTQ1cxcFkzSnZjMjltZEEiLCJhdWQiOiJhcmdvLWNkIiwiZXhwIjoxNzEyMzA1NDk1LCJpYXQiOjE3MTIyMTkwOTUsImF0X2hhc2giOiI1czViaUFRNGVUNXAzWVBFWElzOGFRIiwiY19oYXNoIjoiMVZfVnRnakxCS3pjZHNRMFVQSHotdyIsImVtYWlsIjoiYW5yZWNpb0BlbXRtYWRyaWQuZXMiLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZ3JvdXBzIjpbIlJlZGlzZcOxbyBkZSBQcm9jZWRpbWllbnRvcyBGdW5jaW9uYWxlcyIsIlZhbG9yYWNpw7NuIG9mZXJ0YXMgQnVzIGEgZGVtYW5kYSIsIkZvbmRvcyBFdXJvcGVvcyAoaW50ZXJubyBEaXIuIFRlY25vbG9nw61hKSIsImxpc3RhLlBIUi5wcm9jZXNvcy5HRVgiLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fYWdlbmNpYSIsIlByb3llY3RvcyBldXJvcGVvcywgaW50ZXJuYWNpb25hbGVzIHkgb3RyYXMgY29sYWJvcmFjaW9uZXMiLCJMaXN0YS5wbGFuaWxsYVNhbGlkYSIsIkxpc3RhLnBldGljaW9uZXMuYXBsbmVnb2Npb19nZXMtaW5jaWRlbmNpYXMiLCJPcGVyYXRpb25zIiwiTWFycXVlc2luYXMgNC4wIiwiQ2FyZGlvTUFEIChQdWVzdGEgZW4gTWFyY2hhKSIsIlZpc29yIGRlIFZpZGVvV2FsbCIsIkxBTlpBTUlFTlRPUyBZIFBSRVNFTlRBQ0lPTkVTIiwiQmljaVBBUksiLCJEZW1hbmRhIGVuIFRpZW1wbyBSZWFsIiwiVmlzdWFsaXphY2lvbmVzIFBvd2VyQkkiLCJTbWFydCBCdXMgTWFkcmlkIC0gQXV0b2LDunMgYSBsYSBkZW1hbmRhIiwiQXBhcmNhbWllbnRvcyBEaXN1YXNvcmlvcyIsIkxlbmd1YWplIE5hdHVyYWwgeSBCb3RzIiwiTUFEUklEIE1PQklMSVRZIDM2MCIsIkxpc3RhLkNhbGlkYWQiLCJsaXN0YS5pbnRlZ3JpYS5hcHAubmVnb2NpbyIsImxpc3RhLnVzdWFyaW9zLmRlc2Fycm9sbG8iLCJsaXN0YS5wYW5kb3JhLkdvb2dsZVRyYW5zaXQiLCJBY3VlcmRvcyBDb2xhYm9yYWNpw7NuIEJpY2lNYWQiLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fc2llIiwiUGFzYXJlbGEgZGUgUGFnb3MgRU1UIiwiTVBBU1MiLCJsaXN0YS5pbmNpZGVuY2lhcy5iaWNpbWFkIiwiRm9ybWFjacOzbiBIVE1MK0NTUytKUyIsIlByb3RlY2Npw7NuIGRlIERhdG9zIGVuIEVNVCBNYWRyaWQiLCJGb3JtYWNpw7NuIiwiTWVkaW8gQW1iaWVudGUiLCJTb2NpYWwgTWVkaWEiLCJHb2JpZXJubyBkZWwgRGF0byAoRU1UIC0gRGVzaWRlZGF0dW0pIiwiVGVjbm9sb2fDrWFzIGRlIE5lZ29jaW8iLCJHZXN0acOzbiBkZSAgbGEgRXhwbG90YWNpw7NuIiwiTVdhbGxldCB5IFNESyAgSU5FVFVNK1NPTFVTT0ZUK0VNVCIsIkVzdHVkaW8gVHJhemFiaWxpZGFkIENsaWVudGVzIiwiTXkgIFNlbGYgVGVhbSIsImxpc3RhLmluY2lkZW5jaWFzLlNQTyIsIlNBRU5leHQiLCJMaXN0YS5yZXN1bWVuQklUIiwibGlzdGEucGFuZG9yYS5FbGV2b24iLCJHcsO6YXMgXyBMaXF1aWRhY2nDs24gYXV0b23DoXRpY2EiLCJsaXN0YS5wYW5kb3JhLmVudmlvcy5DUlRNIiwiZUNvbW1lcmNlIE1wYXkiLCJ1QXp1cmVfQWRvYmVTaWduIiwiVGVjbm9sb2fDrWEgeSBTaXN0ZW1hcyBkZSBJbmZvcm1hY2nDs24iLCJCaWNpQk9YIiwiRWxlY3Ryby1FTVQiLCJBcGFyY2FiaWNpcyIsIkV4cGVyaWVuY2lhIGRlIFVzdWFyaW8iLCJtdVNlbmRBc19tb2JpbGl0eWxhYnMiLCJSZW5vdmFjacOzbiBCaWNpTWFkIEludGVncmFsIiwiTGlzdGEuSmVmZXMuU2VydmljaW8iLCJNYWFzNEFsbCIsIlVzdWFyaW9zX0FjY2Vzb19FeHRlcm5vXzM2NSIsIkNPTUlUw4kgU0VHVVJJREFEIERFIExBIElORk9STUFDScOTTiBZIFBST1RFQ0NJw5NOIERFIERBVE9TIiwiQnVzUmFwaWRUcmFuc2l0IiwiTGlzdGEuR0lTIiwidU9mZmljZTM2NV9Hcm91cHNfTWFuYWdlcnMiLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fZ2V4Y29uIiwiU2VndWltaWVudG8gVGFyZWFzIG90cmFzIERpcmVjY2lvbmVzIEVNVCIsInVBenVyZV9EZXZ0cm9uIiwiRHJlYW0gVGVhbSIsIkVzdHJhdGVnaWFzIGUgaW5pY2lhdGl2YXMgZGUgRGF0b3MiLCJTb2x1c29mdC1FTVQgVGFyZWFzIHkgRG9jdW1lbnRhY2nDs24iLCJJbmNpZGVuY2lhcyBCaWNpTWFkIiwiTW9iaWxpdHkgTWFkcmlkIiwiTW9kZWxvIFByZWRpY3Rpdm8gT2N1cGFjacOzbiBkZWwgQlVTIiwibGlzdGEucmVzcG9uc2FibGVzLmRlcGFydGFtZW50byIsIkNvbnZvY2F0b3JpYXMgRm9uZG9zIHkgQXl1ZGFzIChOZXh0R2VuLCBGRURFUi4uLikiLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fY3VhZHJvc2JkciIsIkNvbmN1cnNvIGRlIGlkZWFzIGRpc2XDsW8gbnVldmEgYXBwIiwibGlzdGEuZGlyZWNjaW9uLnRlY25vbG9naWEiLCJTaXN0ZW1hIGRlIEluZGljYWRvcmVzIiwiTGlzdGEuaW5mb3JtZXMuaW5jaWRlbmNpYXMuQklUIiwibGlzdGEucGFuZG9yYS5zZXJ2aWNpb3Mud2ViIiwibGlzdGEuYXl1ZGFudGVzLnRlY25pY29zIiwiT3JnYW5pemFjacOzbiAtIERpcmVjY2nDs24gZGUgVGVjbm9sb2fDrWEgZSBJbm5vdmFjacOzbiIsIlBsYW5pZmljYWNpw7NuIHkgQ3VhZHJvcyIsIkF1dG9iw7pzIGEgZGVtYW5kYSAtIFdvbmRvIFx1MDAyNiBFTVQiLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fZ2V4YnciLCJHSVMgLSBTaXN0ZW1hIGRlIEluZm9ybWFjacOzbiBHZW9ncsOhZmljYSIsIkxpc3RhLkNsYXNpZmljYWNpb25CSVQiLCJWaXNvciArIEtpYmFuYSAvLyBCaWNpTUFEIiwibGlzdGEuZ2VzdGlvbi5kZXNhcnJvbGxvLnNvZnR3YXJlIiwiQmljaU1BRCIsIkNlbnRyb3MgZGUgT3BlcmFjaW9uZXMiLCJDQVJESU9NQURfVGVjbm9sb2fDrWEiLCJsaXN0YS5zaXN0ZW1hcy5FTVQiLCJBdXRvYnVzZXMgeSBUZWNub2xvZ8OtYSIsIkJQTSBTZWd1aW1pZW50byBIYWJiZ" +
61+
"XIrRU1UIiwiSG9qYSBkZSBydXRhIGVsZWN0csOzbmljYSIsInVBenVyZV9Hb29kSGFiaXR6IiwiQXBsaWNhY2lvbmVzIGRlIE5lZ29jaW8iLCJsaXN0YS5heXVkYW50ZXMudGVjbmljb3MucHJpbmNpcGFsZXMiLCJMaXN0YS5zZGEiLCJUYXJqZXRhIGRlIEVtcGxlYWRvIiwiR2VzdG9yIGRlIENvbnRlbmlkb3MgeSBDYW1wYcOxYXMgcGFyYSBBcHBzIiwiTGlzdGEudG9kb3MiLCJHZXN0acOzbiBkZSBsYSBFeHBsb3RhY2nDs24iLCJMaXN0YS5wZXRpY2lvbmVzLmFwbG5lZ29jaW9fc21zIiwibGlzdGEuYXBsaWNhY2lvbmVzLm5lZ29jaW8iLCJsaXN0YS5kaXZpc2lvbi5zaXN0ZW1hcyIsInVPZmZpY2UzNjVfRTNfVklQIiwiRGlyZWNjacOzbiBkZSBUZWNub2xvZ8OtYSBlIElubm92YWNpw7NuIiwiTmV3IEJpY2lNQUQiLCJsaXN0YS5hdmlzb3MuR0VYQ09OIiwiTWlncmFjacOzbiBNTSBFTVQiLCJsaXN0YS5leGNsYWltZXIucGFjaWZpY28iLCJQcm95ZWN0byBTUE8iLCJJbnRlZ3JhY2lvbmVzIGVuIEFwYXJjYW1pZW50b3MiLCJUcmFzbGFkbyBkZSBMYSBFbGlwYSBhIEZ1ZW5jYXJyYWwiLCJFdm9sdWNpw7NuIGVjb3Npc3RlbWEgTUIzNjAiLCJEZXNpZ24gSWRlYXMiLCJSZWR1Y2Npw7NuIGRlbCBTZXJ2aWNpbyIsIkVzdGFiaWxpemFjacOzbiBiaWNpbWFkIiwiU2VndWltaWVudG8gdGVtYXMgQ1JUTSIsImxpc3RhLmNhbGlkYWQuYWlyZSIsIkRldmVsb3BtZW50IiwiUHJvdG9jb2xvIGRlIFBhbmVsZXMgRXh0ZXJpb3JlcyIsIk51ZXZhIENvbnNvbGEgIGRlIENvbmR1Y3RvciIsImxpc3RhLkJXIiwiQ29tcGV0aXRpdmUgSW50ZWxsaWdlbmNlIiwiRGlzZcOxbyBkZSBudWV2YSBDb25zb2xhIGRlIENvbmR1Y3RvciIsImxpc3RhLlBIUi5wcm9jZXNvcy5jdWFkcm9zIiwibXVTZW5kQXNfb3BlbmRhdGEiLCJCaWNpbWFkLU1QYXNzIiwiRnJlZS1mbG9hdGluZyBBdmFuemEgQmlrZSAtIEVNVCJdLCJuYW1lIjoiQW5kcsOpcyBSZWNpbyBNYXJ0w61uIn0.nlp-RKisxYe24vK44k18Eqi44XKIyk0G6cYc5YTmP4B6AD1eV0vMU9YCzEcmTpgp4t1LroYX9Kjox6tOgY1EY6XbbCxJpJ8w9aXP-mMXH5BaiHP1nZVKCWCqwaKxQAwTq9qI30-NedwsPqNOC3zd7xQPKvt3leBv59mdROVV47jfiX2BptJ5vD2qC-jk9A47FngzzNrvForIqgmE2svUgslGsnE7ywx3D28UHKFhDrD-rHyIeOHXDwgCokMILDez9-P-9k9GYJVoBanHSblYzQZjKIjtwOO_9obW9iVb1t6GNk_8co8YaYUHL8TN4g95_UBI6uWHN5CK-xxxyyyzzz"
62+
verifySplitAndJoinToken(token, tt)
63+
})
64+
65+
t.Run("5 key vals in claims", func(tt *testing.T) {
66+
token := generateJWTToken(5)
67+
verifySplitAndJoinToken(token, tt)
68+
})
69+
70+
t.Run("10 key vals in claims", func(tt *testing.T) {
71+
token := generateJWTToken(10)
72+
verifySplitAndJoinToken(token, tt)
73+
})
74+
75+
t.Run("20 key vals in claims", func(tt *testing.T) {
76+
token := generateJWTToken(20)
77+
verifySplitAndJoinToken(token, tt)
78+
})
79+
80+
t.Run("100 key vals in claims", func(tt *testing.T) {
81+
token := generateJWTToken(100)
82+
verifySplitAndJoinToken(token, tt)
83+
})
84+
85+
}

oidc/oidc.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ type ClientApp struct {
107107
// cache holds temporary nonce tokens to which hold application state values
108108
// See http://tools.ietf.org/html/rfc6749#section-10.12 for more info.
109109
cache OIDCStateStorage
110-
//used to verify user by email before setting cookie
110+
// used to verify user by email before setting cookie
111111
userVerifier UserVerifier
112112

113113
RedirectUrlSanitiser RedirectUrlSanitiser
@@ -337,9 +337,9 @@ func isValidRedirectURL(redirectURL string, allowedURLs []string) bool {
337337
// scheme and host are mandatory to match.
338338
if b.Scheme == r.Scheme && b.Host == r.Host {
339339
// If path of redirectURL and allowedURL match, redirectURL is allowed
340-
//if b.Path == r.Path {
340+
// if b.Path == r.Path {
341341
// return true
342-
//}
342+
// }
343343
// If path of redirectURL is within allowed URL's path, redirectURL is allowed
344344
if strings.HasPrefix(path.Clean(r.Path), b.Path) {
345345
return true
@@ -552,7 +552,7 @@ func AppendClaimsAuthenticationRequestParameter(opts []oauth2.AuthCodeOption, re
552552
if len(requestedClaims) == 0 {
553553
return opts
554554
}
555-
log.Infof("RequestedClaims: %s\n", requestedClaims)
555+
log.Infof("RequestedClaims: %v\n", requestedClaims)
556556
claimsRequestParameter, err := createClaimsAuthenticationRequestParameter(requestedClaims)
557557
if err != nil {
558558
log.Errorf("Failed to create OIDC claims authentication request parameter from config: %s", err)

0 commit comments

Comments
 (0)