Skip to content

Commit 58c06ee

Browse files
authored
Portal: Various bug fixes #5792
Portal: Various bug fixes
2 parents 49f358b + 7079e02 commit 58c06ee

5 files changed

Lines changed: 222 additions & 3 deletions

File tree

.vettedpositions

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,8 @@
316316
/pkg/lib/session/test/context.go:85:35: requestcontext
317317
/pkg/lib/workflow/intl_middleware.go:16:41: requestcontext
318318
/pkg/portal/csp_middleware.go:16:39: requestcontext
319-
/pkg/portal/session/middleware_session_info.go:52:9: requestcontext
320-
/pkg/portal/session/middleware_session_info.go:204:36: requestcontext
319+
/pkg/portal/session/middleware_session_info.go:54:9: requestcontext
320+
/pkg/portal/session/middleware_session_info.go:220:36: requestcontext
321321
/pkg/portal/session/middleware_session_required.go:12:38: requestcontext
322322
/pkg/portal/transport/admin_api_handler.go:40:9: requestcontext
323323
/pkg/portal/transport/graphql_handler.go:41:29: requestcontext

pkg/lib/config/secret_update_instruction.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/lestrrat-go/jwx/v2/jwk"
99

10+
"github.com/authgear/authgear-server/pkg/api/apierrors"
1011
corerand "github.com/authgear/authgear-server/pkg/util/rand"
1112
"github.com/authgear/authgear-server/pkg/util/setutil"
1213
)
@@ -769,6 +770,10 @@ func (i *SAMLIdpSigningSecretsUpdateInstruction) delete(currentConfig *SecretCon
769770
newCertificates = append(newCertificates, cert)
770771
}
771772

773+
if len(newCertificates) == 0 {
774+
return nil, apierrors.NewInvalid("cannot delete the last SAML IdP signing certificate")
775+
}
776+
772777
credentials.Certificates = newCertificates
773778

774779
var data []byte

pkg/lib/config/testdata/secret_update_instruction.yaml

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,55 @@ updateInstructionJSON: |-
988988
}
989989
}
990990
---
991+
name: delete-last-saml-idp-signing-secret
992+
error: "cannot delete the last SAML IdP signing certificate"
993+
currentSecretConfigYAML: |-
994+
secrets:
995+
- key: saml.idp.signing
996+
data:
997+
certificates:
998+
- certificate:
999+
pem: |
1000+
-----BEGIN CERTIFICATE-----
1001+
MIIC1TCCAb2gAwIBAgIRAJpxx1DW2ObGLT5lUpXARWkwDQYJKoZIhvcNAQELBQAw
1002+
GzEZMBcGA1UEAxMQbXktYXBwLmxvY2FsaG9zdDAgFw0yNDA4MDkwODA3MzlaGA8y
1003+
MDc0MDcyODA4MDczOVowGzEZMBcGA1UEAxMQbXktYXBwLmxvY2FsaG9zdDCCASIw
1004+
DQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAN83SCP6m3ayNriEX6VLiwCqoIHu
1005+
E1d2vFwULyUWOjinI3olWWkA1txAZu2e0Rm+Zslq2sWx/HZ5e83NCzyLQ8aaG1JQ
1006+
OtpbxV2IOybOonveZr1qszvs+1ofGw9sW6AZa7vhH9HhuDqZnM6ArsC7E/D03D4x
1007+
J/2hb6uVj9zHb+Cx4vh1nAnBXXwOSIuo1Jm4a0vZHFs8HT2gmX31K/5hhJuchqiH
1008+
ptqerf0OHq/Zyx+v40oj3/cFwGAJ291z6kv318bfjBhZTdQ2ovbnFnU9NfQ02IgW
1009+
tSj1Grr8dAp5aIDZvgvvYg/m+FnyMqrSU5s0NIyn13tqipZgN4YUk8CUkCECAwEA
1010+
AaMSMBAwDgYDVR0PAQH/BAQDAgeAMA0GCSqGSIb3DQEBCwUAA4IBAQAVuZEbgLi0
1011+
gzKy5x+L1j+uQMFdY4taFWGdTF7gZx/hw2YpKakPSCl/Sb+624u3+XhQSzByjt7m
1012+
0yGhAml5aLQ+y7jOAwagL0pWhK/AW6kZKU2lz36J+T8LTzq3YOFBHrLTJ58ZcWKe
1013+
kgwAWDr8Uj9BgxnQWF4Rwu8yAP8POV4E6aIajalFK3tNdyGaXIS5rSHGd/QKuJNW
1014+
eCHF7sKGUSTw3p3MADXGkDykUCuXevyNACH6opOLrDCHr/uEEFmSTVf5zlIeSk+Y
1015+
EMgvAyAtQw4fi3WItQNOSLm+01kxkCC1SF+LXTSUPMsLOnX++WJ4u4VJTMfqrh6d
1016+
UgPkRnolBQXT
1017+
-----END CERTIFICATE-----
1018+
key:
1019+
d: Q1QxdmjJK-xlhRKopvs6WmEdORPO0bsG5d9QuScTOLNxiA5sAbg3Fwh2A4a8G7_K0UZh5u_g7vuF1HXcxc5cCzPAuftpRSQZTDUjYnlBmYtkAg6FMbYxuuVXDDfrC0efr3QgXPZw03wySDeYfpnvUecJ59h7740aWe8rDbU6kdOGIvEblHttmKdhazi4c6pJPo8zmEn5UAq0_W_2_0V0CSVsDkvm2mOj_fSXOy9g-tohq03S16wh22QLIFLaN91wpvEMw2hKAHYlENkeBQbS3pi08xVPgbZ2zZ_TVj2ZDh7ozutzFIT73vjUYvGDmiF3jBoj7juZ3Fc34t49tiJ4YQ
1020+
dp: 0lnRUGItEYTqt11_aibuurfAIjpr2sfd7ECW-XvjUIIDYPWNOgIs1mHUGnkAodArA_k6oFk1OA6nInaL0zb_86T3kNyCoZc9-OEQHk1XyeX8lFDTCXc_atQcrScLyKEcIZMrA73R63JVbjOamljTVMT1awpKSLOB3zwdnIw3RTU
1021+
dq: cTfEQe9kJTx1DpdT381eqiMKJjS9ZKFK1A1jTTUXbPNp2-UXeS9eUkB5HsCDxzt-I2VPL22bfywb6eRFkLvRlj6Q800oz-ZqSl-Sjih4ku6X3zY4ofCTHWV16GihvBnNTrW9DR_2B23N013siJN4vOeGCrr5aC1YSRicDGsD0Os
1022+
e: AQAB
1023+
kid: "-jeTn3EEV5bJUOMDQU04zLlkWFZK3BDODkfh40FX8K0"
1024+
kty: RSA
1025+
"n": 3zdII_qbdrI2uIRfpUuLAKqgge4TV3a8XBQvJRY6OKcjeiVZaQDW3EBm7Z7RGb5myWraxbH8dnl7zc0LPItDxpobUlA62lvFXYg7Js6ie95mvWqzO-z7Wh8bD2xboBlru-Ef0eG4OpmczoCuwLsT8PTcPjEn_aFvq5WP3Mdv4LHi-HWcCcFdfA5Ii6jUmbhrS9kcWzwdPaCZffUr_mGEm5yGqIem2p6t_Q4er9nLH6_jSiPf9wXAYAnb3XPqS_fXxt-MGFlN1Dai9ucWdT019DTYiBa1KPUauvx0CnlogNm-C-9iD-b4WfIyqtJTmzQ0jKfXe2qKlmA3hhSTwJSQIQ
1026+
p: 8aS-s6s7i4TQ4C-hsYlp7zXw6eOdH5kniJhwVc9TUBYPG8xn7Hf3kP8RnlNaFy0MEkqSj-E5Zex8Uikvh0Asawj2Seo-SwSaHLJGjin5eyXyWdwd4LBfWvSOII78vaMQCBwhBYXqzsaKKDqNxXJ_DSYpKE21OCBIvglovs7POsM
1027+
q: 7HpDnji4juHv4v2D7fZ5anZawLgXxQy_czZ4qGbW1se8LZVCyEpGKbbocGjYb4qv4Rb8-qVnUmBp6wiFhP4FH9AE-c7vA4bHCIIwqDo2Dj0RPPBsan_Cf5e5bMlHoeiQHzlpbW0u0ZEQ-McCeejSwyU0MbZC8c008kJ4pSCYs0s
1028+
qi: ImK8o6QUELQ7f0R3VeEiohN7J2OHDvaN0LKnUH9ySb3nOV2x_gzgGTWCidLaINHic5-bu6iCkwDZkUFLjJyXJoic3s6r-oJx9yiaqfxJu7FTSes_aXYB6r74xeQELBDyWKWdnV-1_qo9nzVa-1ouqplMyfHIPk9YdDWDFxmE8uc
1029+
newSecretConfigYAML: ""
1030+
updateInstructionJSON: |-
1031+
{
1032+
"samlIdpSigningSecrets": {
1033+
"action": "delete",
1034+
"deleteData": {
1035+
"keyIDs": ["-jeTn3EEV5bJUOMDQU04zLlkWFZK3BDODkfh40FX8K0"]
1036+
}
1037+
}
1038+
}
1039+
---
9911040
name: set-saml-sp-signing-secret
9921041
error: null
9931042
currentSecretConfigYAML: |-

pkg/portal/session/middleware_session_info.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/http"
77
"net/url"
8+
"strings"
89
"time"
910

1011
"github.com/lestrrat-go/jwx/v2/jwk"
@@ -15,6 +16,7 @@ import (
1516
portalconfig "github.com/authgear/authgear-server/pkg/portal/config"
1617
"github.com/authgear/authgear-server/pkg/util/clock"
1718
"github.com/authgear/authgear-server/pkg/util/duration"
19+
"github.com/authgear/authgear-server/pkg/util/jwtutil"
1820
)
1921

2022
var simpleCache = cache.New(5*time.Minute, 10*time.Minute)
@@ -73,7 +75,21 @@ func (m *SessionInfoMiddleware) jwtToSessionInfo(jwkSet jwk.Set, header http.Hea
7375
// Zero value means invalid session.
7476
sessionInfo = &model.SessionInfo{}
7577

76-
token, err := jwt.ParseHeader(header, "Authorization",
78+
authorization := header.Get("Authorization")
79+
const bearerPrefix = "Bearer "
80+
if !strings.HasPrefix(authorization, bearerPrefix) {
81+
return
82+
}
83+
rawToken := authorization[len(bearerPrefix):]
84+
85+
// Only accept JWT access tokens (typ: "at+jwt"). Reject id_tokens (typ: "JWT")
86+
// and any other token type to prevent token confusion.
87+
hdr, _, err := jwtutil.SplitWithoutVerify([]byte(rawToken))
88+
if err != nil || hdr.Type() != "at+jwt" {
89+
return
90+
}
91+
92+
token, err := jwt.ParseString(rawToken,
7793
jwt.WithVerify(true),
7894
jwt.WithKeySet(jwkSet),
7995
jwt.WithClock(jwtClock{m.Clock}),
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package session
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
"time"
7+
8+
"github.com/lestrrat-go/jwx/v2/jwa"
9+
"github.com/lestrrat-go/jwx/v2/jwk"
10+
"github.com/lestrrat-go/jwx/v2/jws"
11+
"github.com/lestrrat-go/jwx/v2/jwt"
12+
. "github.com/smartystreets/goconvey/convey"
13+
14+
"github.com/authgear/authgear-server/pkg/api/model"
15+
"github.com/authgear/authgear-server/pkg/util/clock"
16+
"github.com/authgear/authgear-server/pkg/util/jwtutil"
17+
)
18+
19+
const testPrivateKeyPEM = `-----BEGIN PRIVATE KEY-----
20+
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC89eQDeH8icj6j
21+
1DUHTXKyhFkYOVrOVLA4xflDwqAuw5IrJQNgIjTsBZXrR1rh4BSBsjoE0ToH+/Da
22+
MfyAicQpv7QPI4pM8a/a3SY+rlr4j4LzFtchUvBMcGbSZZqKINBtxpAsFLPGFnwF
23+
NrxXIwrxE79cgY+g1KcmF8twqDmmash6fMoOeU8MTa8Q9Z7wTzhySeeZlBVFtvJp
24+
79Wqe75dtp0pe6E6ujavVjPifj2Msdl9RW7KhJsttgGhMGR2Jp07nAIBT150qX0G
25+
3gu0G5ILbgxcrhYZYK5fk/u6MQ0sAyXwS+fmppsPmYw6UVYlS2UGnaJlCE7Ml0e2
26+
yyEyrbmnAgMBAAECggEARX7NsDUV1O5deVVnd1sVjvA78DvP2Miu0wKErVYcIXbO
27+
AE4pkqah/hgDzjc9BouqHxUUX4cvp5YSO71cl02TtqMJrvOsPqY4ve7NzQnE7Vui
28+
lpLU5i2hsQs51bGGh7yPy3/WsE+g2n6UeDpsREPgF0/i9ju0PjtXihwAN1u3cCt9
29+
t9CsSGliHqQX9uO7o92yN+aROKEbw3x3gKpRJ/Gv3fQcVR01cXvaBrtdEb6kEVEB
30+
WBlCA0kmRc/H7jVYGcWqalLDjj99Pox47PLUigyJsNxJmMD881Ihah4zEQMpX7pW
31+
eRuyISTAA+i0MXO8+bypE6trglF8YQH6JTcVLTz70QKBgQDngFYD0gAqB41vMpmQ
32+
TGSr16qs63Q9QD0Ot9ZkSvYY745HvK7syLq6FLZl5Qz/f45NQ99BQtkGDZjE8sn9
33+
W4V7/yA8xzNP+xmvdqsoOAcIO4j8W34dA6gS+z4h2u98LpqV9Q6ehbrZCPB8/MSn
34+
1QTnbINGw1ZCxfj6olN7ppaZaQKBgQDQ9RIHxHVhDHWpa83Fgf0oDve2UXD5YDsZ
35+
Axu6cYQOGCM7h0WxwDViIUuieWortYvGq8K1IlqfaDWlo5BHRXozmakMpJ4K8sBW
36+
F8TWn7PYw9cPH2XuZZHPnPiYkkhe0SoAifa3tk4bgyR5txOjdCr5L3ZFWfM7Vmkp
37+
hL2M7JTIjwKBgDXyShkJzs/8gpDvEan2o18IGtXA6I19cr0DSgqFDWQyLs24wmqb
38+
PCgwu3BzN9wyNU78CgKDOV+Xu4npqfhIY4rJoRGIugRhV1L0LF5q7/iTJxDnoTPR
39+
rlD+CzSIeFZP5eYb/RQjxa7dzmzR2mHh2gqz1sOesXNN/v8o5Jtj7qRBAoGAEibH
40+
yy7wt156th3sQRT6pckvEYJfmvoWCCUx+m8z9nl4TgqBLmCxAnY7+MAtTeC2ZKq0
41+
/kEeuCw4RMxBkz9gzyyw960xIWhW9uOXsMEswU651tF2bFAca3mKSs6iRMJMsMFL
42+
Ukge3tr0hzI1HYTQ2taZooqey2/FMNscECrY/dcCgYEAuhBfEof+DCeuLmKgvks+
43+
Idv5Ky2ZIR49L8VxCy7K+BXhr2vnKX6itlVDQVVpNIphdLHXQK6CNr8Ko5WinZHu
44+
gouLseU4p4zh8vYZcgPyqlLEdkygMCN0b0+HVaBTs0jlLGbvTC0Oiz69umYMe+5g
45+
eZDnqWNf7mYPdP5mO5iTtMw=
46+
-----END PRIVATE KEY-----
47+
`
48+
49+
// buildTestJWKSet parses testPrivateKeyPEM into a JWK set usable for both
50+
// signing (private key) and verification (public key).
51+
func buildTestJWKSet(t *testing.T) (privateJWKSet jwk.Set, publicJWKSet jwk.Set) {
52+
t.Helper()
53+
set, err := jwk.Parse([]byte(testPrivateKeyPEM), jwk.WithPEM(true))
54+
if err != nil {
55+
t.Fatal(err)
56+
}
57+
privKey, _ := set.Key(0)
58+
_ = privKey.Set(jwk.KeyIDKey, "test-key-1")
59+
_ = privKey.Set(jwk.AlgorithmKey, jwa.RS256.String())
60+
61+
pubSet, err := jwk.PublicSetOf(set)
62+
if err != nil {
63+
t.Fatal(err)
64+
}
65+
return set, pubSet
66+
}
67+
68+
// signToken builds and signs a JWT with the given typ header and claims.
69+
func signToken(t *testing.T, privJWKSet jwk.Set, typ string, sub string, issuedAt time.Time, expiry time.Time) string {
70+
t.Helper()
71+
72+
claims := jwt.New()
73+
_ = claims.Set(jwt.SubjectKey, sub)
74+
_ = claims.Set(jwt.IssuedAtKey, issuedAt.Unix())
75+
_ = claims.Set(jwt.ExpirationKey, expiry.Unix())
76+
_ = claims.Set(string(model.ClaimUserIsAnonymous), false)
77+
_ = claims.Set(string(model.ClaimUserIsVerified), true)
78+
_ = claims.Set(string(model.ClaimUserCanReauthenticate), true)
79+
_ = claims.Set(string(model.ClaimAuthgearRoles), []any{})
80+
81+
privKey, _ := privJWKSet.Key(0)
82+
83+
hdr := jws.NewHeaders()
84+
_ = hdr.Set("typ", typ)
85+
86+
signed, err := jwtutil.SignWithHeader(claims, hdr, jwa.RS256, privKey)
87+
if err != nil {
88+
t.Fatal(err)
89+
}
90+
return string(signed)
91+
}
92+
93+
func makeHeader(token string) http.Header {
94+
h := http.Header{}
95+
h.Set("Authorization", "Bearer "+token)
96+
return h
97+
}
98+
99+
func TestJWTToSessionInfo(t *testing.T) {
100+
Convey("jwtToSessionInfo", t, func() {
101+
privJWKSet, pubJWKSet := buildTestJWKSet(t)
102+
103+
now := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
104+
m := &SessionInfoMiddleware{
105+
Clock: clock.NewMockClockAtTime(now),
106+
}
107+
108+
Convey("valid at+jwt access token returns valid session", func() {
109+
token := signToken(t, privJWKSet, "at+jwt", "user123", now.Add(-1*time.Minute), now.Add(1*time.Hour))
110+
info := m.jwtToSessionInfo(pubJWKSet, makeHeader(token))
111+
So(info.IsValid, ShouldBeTrue)
112+
So(info.UserID, ShouldEqual, "user123")
113+
So(info.UserVerified, ShouldBeTrue)
114+
So(info.UserCanReauthenticate, ShouldBeTrue)
115+
})
116+
117+
Convey("id_token with typ JWT is rejected", func() {
118+
token := signToken(t, privJWKSet, "JWT", "user123", now.Add(-1*time.Minute), now.Add(1*time.Hour))
119+
info := m.jwtToSessionInfo(pubJWKSet, makeHeader(token))
120+
So(info.IsValid, ShouldBeFalse)
121+
})
122+
123+
Convey("missing Authorization header returns invalid session", func() {
124+
info := m.jwtToSessionInfo(pubJWKSet, http.Header{})
125+
So(info.IsValid, ShouldBeFalse)
126+
})
127+
128+
Convey("malformed Authorization header returns invalid session", func() {
129+
h := http.Header{}
130+
h.Set("Authorization", "notabearer")
131+
info := m.jwtToSessionInfo(pubJWKSet, h)
132+
So(info.IsValid, ShouldBeFalse)
133+
})
134+
135+
Convey("expired token returns invalid session", func() {
136+
token := signToken(t, privJWKSet, "at+jwt", "user123", now.Add(-2*time.Hour), now.Add(-1*time.Hour))
137+
info := m.jwtToSessionInfo(pubJWKSet, makeHeader(token))
138+
So(info.IsValid, ShouldBeFalse)
139+
})
140+
141+
Convey("token signed with a different key returns invalid session", func() {
142+
// Sign with our private key but verify against an empty JWK set — no matching key.
143+
emptyPubSet := jwk.NewSet()
144+
token := signToken(t, privJWKSet, "at+jwt", "user123", now.Add(-1*time.Minute), now.Add(1*time.Hour))
145+
info := m.jwtToSessionInfo(emptyPubSet, makeHeader(token))
146+
So(info.IsValid, ShouldBeFalse)
147+
})
148+
})
149+
}

0 commit comments

Comments
 (0)