Skip to content

Commit 6efd769

Browse files
author
Igor Drozdov
committed
Trim secret before signing JWT tokens
With this change we don't rely on the secret to either contain a newline or not contain it.
1 parent 2094323 commit 6efd769

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

client/client_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
)
2121

2222
var (
23-
secret = []byte("sssh, it's a secret")
23+
secret = "sssh, it's a secret"
2424
)
2525

2626
func TestClients(t *testing.T) {
@@ -31,31 +31,45 @@ func TestClients(t *testing.T) {
3131
relativeURLRoot string
3232
caFile string
3333
server func(*testing.T, []testserver.TestRequestHandler) string
34+
secret string
3435
}{
3536
{
3637
desc: "Socket client",
3738
server: testserver.StartSocketHttpServer,
39+
secret: secret,
3840
},
3941
{
4042
desc: "Socket client with a relative URL at /",
4143
relativeURLRoot: "/",
4244
server: testserver.StartSocketHttpServer,
45+
secret: secret,
4346
},
4447
{
4548
desc: "Socket client with relative URL at /gitlab",
4649
relativeURLRoot: "/gitlab",
4750
server: testserver.StartSocketHttpServer,
51+
secret: secret,
4852
},
4953
{
5054
desc: "Http client",
5155
server: testserver.StartHttpServer,
56+
secret: secret,
5257
},
5358
{
5459
desc: "Https client",
5560
caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
5661
server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
5762
return testserver.StartHttpsServer(t, handlers, "")
5863
},
64+
secret: secret,
65+
},
66+
{
67+
desc: "Secret with newlines",
68+
caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
69+
server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
70+
return testserver.StartHttpsServer(t, handlers, "")
71+
},
72+
secret: "\n" + secret + "\n",
5973
},
6074
}
6175

@@ -66,15 +80,15 @@ func TestClients(t *testing.T) {
6680
httpClient, err := NewHTTPClientWithOpts(url, tc.relativeURLRoot, tc.caFile, "", 1, nil)
6781
require.NoError(t, err)
6882

69-
client, err := NewGitlabNetClient("", "", string(secret), httpClient)
83+
client, err := NewGitlabNetClient("", "", tc.secret, httpClient)
7084
require.NoError(t, err)
7185

7286
testBrokenRequest(t, client)
7387
testSuccessfulGet(t, client)
7488
testSuccessfulPost(t, client)
7589
testMissing(t, client)
7690
testErrorMessage(t, client)
77-
testAuthenticationHeader(t, client)
91+
testAuthenticationHeader(t, tc.secret, client)
7892
testJWTAuthenticationHeader(t, client)
7993
testXForwardedForHeader(t, client)
8094
testHostWithTrailingSlash(t, client)
@@ -154,7 +168,7 @@ func testBrokenRequest(t *testing.T, client *GitlabNetClient) {
154168
})
155169
}
156170

157-
func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
171+
func testAuthenticationHeader(t *testing.T, secret string, client *GitlabNetClient) {
158172
t.Run("Authentication headers for GET", func(t *testing.T) {
159173
response, err := client.Get(context.Background(), "/auth")
160174
require.NoError(t, err)
@@ -167,7 +181,7 @@ func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
167181

168182
header, err := base64.StdEncoding.DecodeString(string(responseBody))
169183
require.NoError(t, err)
170-
require.Equal(t, secret, header)
184+
require.Equal(t, secret, string(header))
171185
})
172186

173187
t.Run("Authentication headers for POST", func(t *testing.T) {
@@ -182,7 +196,7 @@ func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
182196

183197
header, err := base64.StdEncoding.DecodeString(string(responseBody))
184198
require.NoError(t, err)
185-
require.Equal(t, secret, header)
199+
require.Equal(t, secret, string(header))
186200
})
187201
}
188202

@@ -193,7 +207,7 @@ func testJWTAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
193207

194208
claims := &jwt.RegisteredClaims{}
195209
token, err := jwt.ParseWithClaims(string(responseBody), claims, func(token *jwt.Token) (interface{}, error) {
196-
return secret, nil
210+
return []byte(secret), nil
197211
})
198212
require.NoError(t, err)
199213
require.True(t, token.Valid)

client/gitlabnet.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,16 +141,15 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
141141
if user != "" && password != "" {
142142
request.SetBasicAuth(user, password)
143143
}
144-
secretBytes := []byte(c.secret)
145-
146-
encodedSecret := base64.StdEncoding.EncodeToString(secretBytes)
144+
encodedSecret := base64.StdEncoding.EncodeToString([]byte(c.secret))
147145
request.Header.Set(secretHeaderName, encodedSecret)
148146

149147
claims := jwt.RegisteredClaims{
150148
Issuer: jwtIssuer,
151149
IssuedAt: jwt.NewNumericDate(time.Now()),
152150
ExpiresAt: jwt.NewNumericDate(time.Now().Add(jwtTTL)),
153151
}
152+
secretBytes := []byte(strings.TrimSpace(c.secret))
154153
tokenString, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(secretBytes)
155154
if err != nil {
156155
return nil, err

0 commit comments

Comments
 (0)