Skip to content

Commit aebf319

Browse files
sujoshuasujoshua
committed
✨ Improve OIDC error feedback and auth page layout
1 parent 2e617aa commit aebf319

5 files changed

Lines changed: 129 additions & 37 deletions

File tree

‎app/appearance/langs/en_US.json‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,9 @@
12821282
"about4": "Open browser",
12831283
"about5": "Access authorization code",
12841284
"about6": "After configuration, it will be used as the access authentication password, leave it blank to close the authentication",
1285+
"accessAuthBypass": "Bypass access authentication",
1286+
"accessAuthBypassTip": "Skip all access code, OIDC checks and mandatory safety checks (not recommended)",
1287+
"accessAuthBypassConfirm": "Are you sure you want to bypass all access authentication and security checks? This will allow anyone to access without credentials.",
12851288
"about7": "Follow system lock screen",
12861289
"about8": "When enabled, the application will be automatically locked when locking the system screen",
12871290
"about11": "Network serving",
@@ -1610,7 +1613,7 @@
16101613
"169": "Uploading data repo file %v/%v",
16111614
"170": "Uploading data repo chunk %v/%v",
16121615
"171": "Uploading data repo reference %s",
1613-
"172": "If you forget the authorization code, please find help <a href=\"https://liuyun.io/article/1686530886208\" target=\"_blank\">here</a>",
1616+
"172": "If you forget the authorization, please find help <a href=\"https://liuyun.io/article/1686530886208\" target=\"_blank\">here</a>",
16141617
"173": "Please enter the access auth code",
16151618
"174": "Unlock access",
16161619
"175": "Please enter the verification code",
@@ -1713,6 +1716,9 @@
17131716
"272": "Unnamed field",
17141717
"273": "Do not create the workspace in the partition root path, please create a new folder as the workspace",
17151718
"274": "This folder contains other files, please create a new folder as the workspace",
1716-
"275": "Cannot open documents created by a newer version. Please upgrade to the latest version and try again"
1719+
"275": "Cannot open documents created by a newer version. Please upgrade to the latest version and try again",
1720+
"276": "OIDC authentication failed, please retry.",
1721+
"277": "OIDC session is invalid or expired, please retry.",
1722+
"278": "OIDC authentication was rejected by policy."
17171723
}
17181724
}

‎app/appearance/langs/zh_CN.json‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,9 @@
12821282
"about4": "打开浏览器",
12831283
"about5": "访问授权码",
12841284
"about6": "配置后作为访问鉴权密码，留空则关闭鉴权",
1285+
"accessAuthBypass": "绕过访问认证",
1286+
"accessAuthBypassTip": "跳过访问授权码、OIDC 等所有认证和强制安全检查（不推荐）",
1287+
"accessAuthBypassConfirm": "确定要绕过所有访问认证和安全检查吗？这将允许任何人无需凭据即可访问。",
12851288
"about7": "跟随系统锁屏",
12861289
"about8": "启用后将会在系统锁屏时自动锁定应用",
12871290
"about11": "网络伺服",
@@ -1610,7 +1613,7 @@
16101613
"169": "正在上传数据仓库文件 %v/%v",
16111614
"170": "正在上传数据仓库分块 %v/%v",
16121615
"171": "正在上传数据仓库引用 %s",
1613-
"172": "如果你忘记了授权码，请在<a href=\"https://ld246.com/article/1649901726096\" target=\"_blank\">这里</a>寻求帮助",
1616+
"172": "如果你忘记了认证方式，请在<a href=\"https://ld246.com/article/1649901726096\" target=\"_blank\">这里</a>寻求帮助",
16141617
"173": "请输入访问授权码",
16151618
"174": "解锁访问",
16161619
"175": "请输入验证码",
@@ -1713,6 +1716,9 @@
17131716
"272": "未命名字段",
17141717
"273": "请勿在分区根路径上创建工作空间，请新建一个文件夹作为工作空间",
17151718
"274": "该文件夹包含了其他文件，请新建一个文件夹作为工作空间",
1716-
"275": "无法打开新版本创建的文档，请升级到最新版本后再试"
1719+
"275": "无法打开新版本创建的文档，请升级到最新版本后再试",
1720+
"276": "OIDC 认证失败，请重试。",
1721+
"277": "OIDC 会话无效或已过期，请重试。",
1722+
"278": "OIDC 认证被策略拒绝。"
17171723
}
17181724
}

‎app/stage/auth.html‎

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,28 +173,66 @@
173173
font-size: 12px;
174174
margin: 16px 0;
175175
}
176+
177+
.action-group,
178+
.auth-buttons {
179+
display: flex;
180+
flex-direction: column;
181+
align-items: center;
182+
}
183+
184+
.action-group {
185+
gap: 10px;
186+
margin-top: 8px;
187+
}
188+
189+
.auth-buttons {
190+
gap: 6px;
191+
width: 100%;
192+
}
193+
194+
.auth-buttons .b3-button {
195+
margin: 6px 0;
196+
}
176197
</style>
177198
</head>
178199
<body>
179200
<div style="-webkit-app-region: drag;height: 32px;width: 100%;position: absolute;top: 0;"></div>
180201
<div style="position: relative;z-index: 2;text-align: center">
181202
<h1 style="margin-bottom: 48px;color:var(--b3-theme-on-background)">{{.workspace}}</h1>
203+
{{if .hasAccessAuthCode}}
182204
<input class="b3-text-field" id="authCode" type="password" placeholder="{{.l0}}"/><br>
183205
<div style="position: relative;width: 240px;margin: 8px auto 0;display: none">
184206
<img id="captchaImg" style="top: 1px;position: absolute;height: 26px;right: 1px;cursor: pointer">
185207
<input id="captcha" class="b3-text-field" placeholder="{{.l3}}">
186208
</div>
209+
{{end}}
187210
<div style="width: 240px; margin: 8px auto; text-align: left; display: flex; align-items: center;">
188211
<input type="checkbox" id="rememberMe" style="margin-right: 8px;">
189212
<label for="rememberMe" style="color: var(--b3-theme-on-surface); font-size: 14px;">{{.l10}}</label>
190213
</div>
191-
<button class="b3-button" onclick="submitAuth()">{{.l1}}</button>
192-
<div class="ft__on-surface">
193-
{{.l2}}
194-
</div>
195-
<button class="b3-button b3-button--white" onclick="exitSiYuan()">{{.l5}}</button>
196-
<div class="ft__on-surface">
197-
{{.l7}}
214+
<div class="action-group">
215+
<div class="auth-buttons">
216+
{{if .hasAccessAuthCode}}
217+
<button class="b3-button" onclick="submitAuth()">{{.l1}}</button>
218+
{{end}}
219+
{{if .oidcEnabled}}
220+
<button class="b3-button" onclick="startOIDC()">{{.oidcProviderName}}</button>
221+
{{end}}
222+
{{if .authError}}
223+
<div class="ft__on-surface" style="color: #d93025;">
224+
{{.authError}}
225+
</div>
226+
{{end}}
227+
<div class="ft__on-surface">
228+
{{.l2}}
229+
</div>
230+
</div>
231+
<div style="height: 8px;"></div>
232+
<button class="b3-button b3-button--white" onclick="exitSiYuan()">{{.l5}}</button>
233+
<div class="ft__on-surface">
234+
{{.l7}}
235+
</div>
198236
</div>
199237
</div>
200238
<div style="overflow: hidden;position: absolute;height: 100%;width: 100%;top: 0;left: 0;">
@@ -487,10 +525,27 @@ <h1 style="margin-bottom: 48px;color:var(--b3-theme-on-background)">{{.workspace
487525
}, 6000)
488526
}
489527

528+
const startOIDC = () => {
529+
const rememberMeElement = document.getElementById('rememberMe')
530+
const url = new URL('/auth/oidc/login', window.location.origin)
531+
const currentUrl = new URL(window.location.href)
532+
const toParam = currentUrl.searchParams.get("to")
533+
if (toParam) {
534+
url.searchParams.set("to", toParam)
535+
}
536+
if (rememberMeElement && rememberMeElement.checked) {
537+
url.searchParams.set("rememberMe", "1")
538+
}
539+
window.location.href = url.toString()
540+
}
541+
490542
const submitAuth = () => {
491543
const inputElement = document.getElementById('authCode')
492544
const captchaElement = document.getElementById('captcha')
493545
const rememberMeElement = document.getElementById('rememberMe')
546+
if (!inputElement || !captchaElement) {
547+
return
548+
}
494549
let code = inputElement.value.trim();
495550
if ("" === code) {
496551
showMessage({{.l9}})
@@ -559,16 +614,18 @@ <h1 style="margin-bottom: 48px;color:var(--b3-theme-on-background)">{{.workspace
559614

560615
const inputElement = document.getElementById('authCode')
561616
const captchaElement = document.getElementById('captcha')
562-
inputElement.focus()
563-
inputElement.addEventListener('keydown', (event) => {
564-
if (event.key === 'Enter') {
565-
submitAuth()
566-
}
567-
})
617+
if (inputElement && captchaElement) {
618+
inputElement.focus()
619+
inputElement.addEventListener('keydown', (event) => {
620+
if (event.key === 'Enter') {
621+
submitAuth()
622+
}
623+
})
568624

569-
captchaElement.previousElementSibling.addEventListener('click', function () {
570-
this.src = `/api/system/getCaptcha?v=${new Date().getTime()}`
571-
})
625+
captchaElement.previousElementSibling.addEventListener('click', function () {
626+
this.src = `/api/system/getCaptcha?v=${new Date().getTime()}`
627+
})
628+
}
572629

573630
// 用于授权页保持连接，避免非常驻内存内核自动退出 https://github.com/siyuan-note/insider/issues/1099
574631
new WebSocket((window.location.protocol === 'https:' ? 'wss' : 'ws') + '://' + window.location.host + '/ws?app=siyuan&id=auth')

‎kernel/model/oidc/oidc.go‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func Login(c *gin.Context, oidcConf *conf.OIDC) {
111111
c.Redirect(http.StatusFound, authURL)
112112
}
113113

114-
func Callback(c *gin.Context, oidcConf *conf.OIDC) {
114+
func Callback(c *gin.Context, oidcConf *conf.OIDC, lang func(int) string) {
115115
if !IsEnabled(oidcConf) {
116116
c.Status(http.StatusNotFound)
117117
return
@@ -120,28 +120,25 @@ func Callback(c *gin.Context, oidcConf *conf.OIDC) {
120120
session := util.GetSession(c)
121121
workspaceSession := util.GetWorkspaceSession(session)
122122
if "" == workspaceSession.OIDC.State || c.Query("state") != workspaceSession.OIDC.State {
123-
logging.LogWarnf("invalid oidc state [ip=%s]", util.GetRemoteAddr(c.Request))
124-
c.Status(http.StatusUnauthorized)
123+
oidcCallbackError(c, workspaceSession, lang, 277, "invalid oidc state", nil)
125124
return
126125
}
127126

128127
code := c.Query("code")
129128
if "" == code {
130-
logging.LogWarnf("missing oidc code [ip=%s]", util.GetRemoteAddr(c.Request))
131-
c.Status(http.StatusUnauthorized)
129+
oidcCallbackError(c, workspaceSession, lang, 277, "missing oidc code", nil)
132130
return
133131
}
134132

135133
p, err := providerInstance(oidcConf)
136134
if err != nil {
137135
logging.LogErrorf("init oidc provider failed: %s", err)
138-
c.Status(http.StatusInternalServerError)
136+
oidcCallbackError(c, workspaceSession, lang, 276, "init oidc provider failed", err)
139137
return
140138
}
141139

142140
if "" != workspaceSession.OIDC.Provider && workspaceSession.OIDC.Provider != p.ID() {
143-
logging.LogWarnf("oidc provider mismatch [ip=%s]", util.GetRemoteAddr(c.Request))
144-
c.Status(http.StatusUnauthorized)
141+
oidcCallbackError(c, workspaceSession, lang, 277, "oidc provider mismatch", nil)
145142
return
146143
}
147144

@@ -150,20 +147,17 @@ func Callback(c *gin.Context, oidcConf *conf.OIDC) {
150147

151148
claims, err := p.HandleCallback(ctx, code, workspaceSession.OIDC.Nonce)
152149
if err != nil {
153-
logging.LogWarnf("oidc callback failed: %s", err)
154-
c.Status(http.StatusUnauthorized)
150+
oidcCallbackError(c, workspaceSession, lang, 276, "oidc callback failed", err)
155151
return
156152
}
157153

158154
if "" == claims.Subject {
159-
logging.LogWarnf("oidc subject missing [ip=%s]", util.GetRemoteAddr(c.Request))
160-
c.Status(http.StatusUnauthorized)
155+
oidcCallbackError(c, workspaceSession, lang, 276, "oidc subject missing", nil)
161156
return
162157
}
163158

164159
if !IsAllowed(oidcConf.Filters, claims) {
165-
logging.LogWarnf("oidc filter rejected [ip=%s]", util.GetRemoteAddr(c.Request))
166-
c.Status(http.StatusUnauthorized)
160+
oidcCallbackError(c, workspaceSession, lang, 278, "oidc filter rejected", nil)
167161
return
168162
}
169163

@@ -182,8 +176,7 @@ func Callback(c *gin.Context, oidcConf *conf.OIDC) {
182176
})
183177

184178
if err = session.Save(c); err != nil {
185-
logging.LogErrorf("save session failed: %s", err)
186-
c.Status(http.StatusInternalServerError)
179+
oidcCallbackError(c, workspaceSession, lang, 276, "save session failed", err)
187180
return
188181
}
189182
logging.LogInfof("oidc auth success [ip=%s, maxAge=%d]", util.GetRemoteAddr(c.Request), maxAge)
@@ -238,3 +231,31 @@ func sanitizeRedirectPath(dest string) string {
238231
parsed.User = nil
239232
return parsed.String()
240233
}
234+
235+
func oidcCallbackError(c *gin.Context, workspaceSession *util.WorkspaceSession, lang func(int) string, msgID int, logMsg string, err error) {
236+
userMsg := lang(msgID)
237+
if "" == userMsg {
238+
userMsg = "OIDC authentication failed, please retry."
239+
}
240+
if 200 < len(userMsg) {
241+
userMsg = userMsg[:200] + "..."
242+
}
243+
244+
if err != nil {
245+
logging.LogWarnf("oidc callback failed: %s [err=%s, ip=%s]", logMsg, err, util.GetRemoteAddr(c.Request))
246+
} else {
247+
logging.LogWarnf("oidc callback failed: %s [ip=%s]", logMsg, util.GetRemoteAddr(c.Request))
248+
}
249+
250+
location := url.URL{Path: "/check-auth"}
251+
queryParams := url.Values{}
252+
if workspaceSession != nil && workspaceSession.OIDC != nil {
253+
to := sanitizeRedirectPath(workspaceSession.OIDC.To)
254+
if "" != to {
255+
queryParams.Set("to", to)
256+
}
257+
}
258+
queryParams.Set("error", userMsg)
259+
location.RawQuery = queryParams.Encode()
260+
c.Redirect(http.StatusFound, location.String())
261+
}

‎kernel/server/serve.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ func serveOIDC(ginServer *gin.Engine) {
423423
oidc.Login(c, model.Conf.OIDC)
424424
})
425425
ginServer.GET("/auth/oidc/callback", func(c *gin.Context) {
426-
oidc.Callback(c, model.Conf.OIDC)
426+
oidc.Callback(c, model.Conf.OIDC, model.Conf.Language)
427427
})
428428
}
429429

@@ -463,6 +463,7 @@ func serveAuthPage(c *gin.Context) {
463463
}
464464
oidcEnabled := oidc.IsEnabled(model.Conf.OIDC)
465465
oidcProviderName := oidc.ProviderLabel(model.Conf.OIDC)
466+
authError := strings.TrimSpace(c.Query("error"))
466467
model := map[string]interface{}{
467468
"l0": model.Conf.Language(173),
468469
"l1": model.Conf.Language(174),
@@ -485,6 +486,7 @@ func serveAuthPage(c *gin.Context) {
485486
"oidcEnabled": oidcEnabled,
486487
"oidcProviderName": oidcProviderName,
487488
"hasAccessAuthCode": "" != model.Conf.AccessAuthCode,
489+
"authError": authError,
488490
}
489491
buf := &bytes.Buffer{}
490492
if err = tpl.Execute(buf, model); err != nil {

0 commit comments

Comments
 (0)