Skip to content

Commit cba0087

Browse files
committed
fix: eliminate 16 SonarCloud security hotspots (S2092/S3330)
Replace cookie-based auth state passing with server-rendered data attributes on the consent page. This eliminates all non-HttpOnly and non-Secure cookie patterns that SonarCloud flags as security hotspots. Previously, the consent page handler set browser cookies (auth_method, *_redirect_url) that JavaScript read on page load. Since JS needed access, these cookies could not be HttpOnly — triggering S3330 hotspots. Cookie deletion calls in callbacks used Secure=false/HttpOnly=false — triggering both S2092 and S3330. Now the Go handler passes AuthMethod and RedirectURL via gin.H template context. The HTML template embeds these as data-* attributes on the Alpine.js root element. JavaScript reads from this.$el.dataset instead of cookies. Changes: - endpoints_oauth.go: Replace 4 SetCookie calls with gin.H template data - endpoints_saml.go: Remove 2 cookie deletion calls (no cookies to delete) - endpoints_oidcrp.go: Remove 2 cookie deletion calls - endpoints_users.go: Remove 4 cookie deletion calls - consent.html: Add data-auth-method and data-redirect-url attributes - consent.js: Read from data attributes, remove getCookie helper
1 parent bf06e78 commit cba0087

File tree

6 files changed

+19
-43
lines changed

6 files changed

+19
-43
lines changed

internal/apigw/httpserver/endpoints_oauth.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,10 @@ func (s *Service) endpointOAuthAuthorizationConsent(ctx context.Context, c *gin.
170170
return nil, err
171171
}
172172

173-
// Set SameSite policy for consent cookies; Secure flag follows TLS configuration
174-
c.SetSameSite(http.SameSiteLaxMode)
175-
secureCookie := s.sessionsOptions.Secure
176-
c.SetCookie("auth_method", authMethod, 900, "/authorization/consent", "", secureCookie, false)
173+
// Collect template data for the consent page instead of setting cookies.
174+
// This avoids non-HttpOnly cookies (S3330) since JavaScript reads auth state
175+
// from data attributes embedded in the rendered HTML.
176+
var redirectURL string
177177

178178
sessionID, ok := session.Get("session_id").(string)
179179
if !ok {
@@ -193,7 +193,7 @@ func (s *Service) endpointOAuthAuthorizationConsent(ctx context.Context, c *gin.
193193
return nil, err
194194
}
195195

196-
c.SetCookie("pid_auth_redirect_url", reply.RedirectURL, 900, "/authorization/consent", "", secureCookie, false)
196+
redirectURL = reply.RedirectURL
197197
session.Set("verifier_context_id", reply.VerifierContextID)
198198
if err := session.Save(); err != nil {
199199
return nil, err
@@ -226,7 +226,7 @@ func (s *Service) endpointOAuthAuthorizationConsent(ctx context.Context, c *gin.
226226
return nil, err
227227
}
228228

229-
c.SetCookie("saml_redirect_url", authReq.RedirectURL, 900, "/authorization/consent", "", secureCookie, false)
229+
redirectURL = authReq.RedirectURL
230230
}
231231

232232
if authMethod == model.AuthMethodOIDC {
@@ -249,10 +249,13 @@ func (s *Service) endpointOAuthAuthorizationConsent(ctx context.Context, c *gin.
249249
return nil, err
250250
}
251251

252-
c.SetCookie("oidc_redirect_url", authReq.AuthorizationURL, 900, "/authorization/consent", "", secureCookie, false)
252+
redirectURL = authReq.AuthorizationURL
253253
}
254254

255-
c.HTML(http.StatusOK, "consent.html", nil)
255+
c.HTML(http.StatusOK, "consent.html", gin.H{
256+
"AuthMethod": authMethod,
257+
"RedirectURL": redirectURL,
258+
})
256259
return nil, nil
257260
}
258261
func (s *Service) endpointOAuthAuthorizationConsentCallback(ctx context.Context, c *gin.Context) (any, error) {

internal/apigw/httpserver/endpoints_oidcrp.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ func (s *Service) endpointOIDCRPCallback(ctx context.Context, c *gin.Context) (a
9696

9797
// VCI mode: redirect browser back to consent page
9898
if reply != nil && reply.VCIRedirectURL != "" {
99-
// Clean up auth cookies before redirect
100-
c.SetCookie("auth_method", "", -1, "/authorization/consent", "", false, false)
101-
c.SetCookie("oidc_redirect_url", "", -1, "/authorization/consent", "", false, false)
10299
c.Redirect(http.StatusFound, reply.VCIRedirectURL)
103100
return nil, nil
104101
}

internal/apigw/httpserver/endpoints_saml.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,6 @@ func (s *Service) endpointSAMLACS(ctx context.Context, c *gin.Context) (any, err
223223
err = nil
224224
s.samlSPService.DeleteSession(ctx, relayState)
225225

226-
// Clean up auth cookies before redirect
227-
c.SetCookie("auth_method", "", -1, "/authorization/consent", "", false, false)
228-
c.SetCookie("saml_redirect_url", "", -1, "/authorization/consent", "", false, false)
229-
230226
// Redirect browser back to the consent page to continue the VCI flow
231227
c.Redirect(http.StatusFound, "/authorization/consent/#/credentials")
232228
return nil, nil

internal/apigw/httpserver/endpoints_users.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,6 @@ func (s *Service) endpointUserCancel(ctx context.Context, c *gin.Context) (any,
207207
return nil, err
208208
}
209209

210-
// Delete all cookies, not just session.
211-
c.SetCookie("auth_method", "", -1, "/authorization/consent", "", false, false)
212-
c.SetCookie("pid_auth_redirect_url", "", -1, "/authorization/consent", "", false, false)
213-
c.SetCookie("oidc_redirect_url", "", -1, "/authorization/consent", "", false, false)
214-
c.SetCookie("saml_redirect_url", "", -1, "/authorization/consent", "", false, false)
215-
216210
client, ok := s.cfg.APIGW.OauthServer.Clients[clientId]
217211
if !ok {
218212
err := errors.New("invalid client_id")

internal/apigw/staticembed/consent.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
<link rel="icon" href="/static/favicon.png" type="image/png">
6464
</head>
6565
<body class="content-fade-in-appear-done content-fade-in-enter-done">
66-
<section class="flex-grow flex flex-col items-center justify-center bg-gray-100 dark:bg-gray-900 min-h-dvh flex flex-col" x-data="app">
66+
<section class="flex-grow flex flex-col items-center justify-center bg-gray-100 dark:bg-gray-900 min-h-dvh flex flex-col" x-data="app" data-auth-method="{{.AuthMethod}}" data-redirect-url="{{.RedirectURL}}">
6767
<div class="w-full flex flex-col items-center gap-7 px-6 py-8 text-black dark:text-white max-w-lg p-8">
6868
<a href="https://www.sunet.se" target="_blank" rel="noopener noreferrer">
6969
<img src="/static/logo.png" alt="Sunet" style="height: 48px;">

internal/apigw/staticembed/consent.js

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,6 @@ const UserDataSchema = v.required(v.object({
3939
redirect_url: v.string(),
4040
}));
4141

42-
/**
43-
* @param {string} name
44-
* @returns {string | null}
45-
*/
46-
function getCookie(name) {
47-
return document.cookie
48-
.split(";")
49-
.find((cookie) =>
50-
cookie.trim().startsWith(`${name}=`),
51-
)
52-
?.split("=")
53-
.pop() || null;
54-
}
55-
5642
/**
5743
* @param {string} key
5844
* @returns {string}
@@ -143,7 +129,7 @@ Alpine.data("app", () => ({
143129
},
144130

145131
setAuthMethod() {
146-
const authMethod = getCookie("auth_method");
132+
const authMethod = this.$el.dataset.authMethod || null;
147133
const validMethods = ["basic", "pid_auth", "saml", "oidc"];
148134

149135
if (!authMethod || !validMethods.includes(authMethod)) {
@@ -226,9 +212,9 @@ Alpine.data("app", () => ({
226212
},
227213

228214
handleLoginSAML() {
229-
const rawRedirectUrl = getCookie("saml_redirect_url");
215+
const rawRedirectUrl = this.$el.dataset.redirectUrl || null;
230216
if (!rawRedirectUrl) {
231-
this.error = "Missing 'saml_redirect_url' cookie";
217+
this.error = "Missing SAML redirect URL";
232218
return;
233219
}
234220
try {
@@ -240,9 +226,9 @@ Alpine.data("app", () => ({
240226
},
241227

242228
handleLoginOIDC() {
243-
const rawRedirectUrl = getCookie("oidc_redirect_url");
229+
const rawRedirectUrl = this.$el.dataset.redirectUrl || null;
244230
if (!rawRedirectUrl) {
245-
this.error = "Missing 'oidc_redirect_url' cookie";
231+
this.error = "Missing OIDC redirect URL";
246232
return;
247233
}
248234
try {
@@ -257,9 +243,9 @@ Alpine.data("app", () => ({
257243
* @param {boolean} immediate - Immediately proceed to 'redirect_uri'
258244
*/
259245
handleLoginPidAuth(immediate = false) {
260-
const rawRedirectUrl = getCookie("pid_auth_redirect_url");
246+
const rawRedirectUrl = this.$el.dataset.redirectUrl || null;
261247
if (!rawRedirectUrl) {
262-
this.error = "Missing 'pid_auth_redirect_url' cookie";
248+
this.error = "Missing PID auth redirect URL";
263249
return;
264250
}
265251

0 commit comments

Comments
 (0)