Collect Android browser sign-ins after the app was stopped
The Android app kept a pending browser sign-in in a thread inside the login screen. Android often kills the process while the browser is in front, so the approval reached the server but nothing collected it, and the app came back signed out with nothing saved. The pending device code is now kept in Prefs and collected whenever the login screen resumes. The app passes return_url=keeporg://device-approved when it starts the sign-in, and the server's approval page, after a password or single sign-on, links back to it. Only the keeporg: scheme is accepted. Against a server older than 0.9, which rejects the field, the app retries without it. Verified on an Android 15 emulator: approval collected after the process was killed, and the fallback against the 0.8 server. Server 0.9.0, Android 0.3.0 (versionCode 3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
ef6598fa15
commit
318bd68908
+22
-2
@@ -4,6 +4,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -187,15 +188,22 @@ func (s *Server) handleTOTPConfirm(w http.ResponseWriter, r *http.Request, u *st
|
||||
|
||||
func (s *Server) handleDeviceStart(w http.ResponseWriter, r *http.Request) {
|
||||
var req struct {
|
||||
Label string `json:"label"`
|
||||
Label string `json:"label"`
|
||||
ReturnURL string `json:"return_url"`
|
||||
}
|
||||
if r.ContentLength > 0 && !readJSON(w, r, &req) {
|
||||
return
|
||||
}
|
||||
if req.ReturnURL != "" && !validReturnURL(req.ReturnURL) {
|
||||
fail(w, http.StatusBadRequest, "bad_request",
|
||||
"return_url must use the "+returnScheme+": scheme")
|
||||
return
|
||||
}
|
||||
d := &store.DeviceAuth{
|
||||
DeviceCode: secret.Token(),
|
||||
UserCode: secret.UserCode(),
|
||||
Label: req.Label,
|
||||
ReturnURL: req.ReturnURL,
|
||||
CreatedAt: time.Now(),
|
||||
ExpiresAt: time.Now().Add(deviceTTL),
|
||||
}
|
||||
@@ -276,11 +284,13 @@ func (s *Server) handleDeviceApprove(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
code := strings.ToUpper(strings.TrimSpace(r.FormValue("code")))
|
||||
var returnURL string
|
||||
err := s.st.Update(func(st *store.State) error {
|
||||
d := st.DeviceByUserCode(code)
|
||||
if d == nil || time.Now().After(d.ExpiresAt) {
|
||||
return errors.New("unknown or expired code")
|
||||
}
|
||||
returnURL = d.ReturnURL
|
||||
u, err := checkPassword(st, r.FormValue("username"), r.FormValue("password"), r.FormValue("totp"))
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -298,10 +308,20 @@ func (s *Server) handleDeviceApprove(w http.ResponseWriter, r *http.Request) {
|
||||
case err != nil:
|
||||
s.renderDevice(w, code, err.Error())
|
||||
default:
|
||||
s.renderDone(w, "Device approved", "You can close this page and return to the application.")
|
||||
s.renderApproved(w, returnURL)
|
||||
}
|
||||
}
|
||||
|
||||
// returnScheme is the only scheme a device may ask to be sent back to. It
|
||||
// names KeepOrg's own clients, so the approval page cannot be turned into a
|
||||
// redirect to anywhere else.
|
||||
const returnScheme = "keeporg"
|
||||
|
||||
func validReturnURL(raw string) bool {
|
||||
u, err := url.Parse(raw)
|
||||
return err == nil && u.Scheme == returnScheme && u.User == nil
|
||||
}
|
||||
|
||||
// changePassword replaces a user's password after proving the current one.
|
||||
// It is the one place a password changes, so the rules live here: the
|
||||
// current password is required even though the caller already holds a
|
||||
|
||||
@@ -452,3 +452,39 @@ func TestNonAdminCannotAdminister(t *testing.T) {
|
||||
t.Fatalf("anonymous list: %d", status)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeviceApprovalLinksBackToTheApp(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
if status, body := h.call("POST", "/api/v1/auth/device/start", "",
|
||||
map[string]string{"label": "phone", "return_url": "https://evil.example/"}); status != http.StatusBadRequest {
|
||||
t.Fatalf("foreign return_url accepted: %d %v", status, body)
|
||||
}
|
||||
status, start := h.call("POST", "/api/v1/auth/device/start", "",
|
||||
map[string]string{"label": "phone", "return_url": "keeporg://device-approved"})
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("device start: %d %v", status, start)
|
||||
}
|
||||
resp, err := h.http.Client().PostForm(h.http.URL+"/device", url.Values{
|
||||
"code": {start["user_code"].(string)}, "username": {"admin"}, "password": {adminPassword}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
page, _ := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if !strings.Contains(string(page), `href="keeporg://device-approved"`) {
|
||||
t.Fatalf("approval page has no link back to the app: %s", page)
|
||||
}
|
||||
|
||||
// A client that asked for nothing gets no link.
|
||||
_, start = h.call("POST", "/api/v1/auth/device/start", "", map[string]string{"label": "laptop"})
|
||||
resp, err = h.http.Client().PostForm(h.http.URL+"/device", url.Values{
|
||||
"code": {start["user_code"].(string)}, "username": {"admin"}, "password": {adminPassword}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
page, _ = io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if strings.Contains(string(page), "Return to KeepOrg") {
|
||||
t.Fatalf("link shown without a return_url: %s", page)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,7 +232,7 @@ func (s *Server) handleOIDCCallback(w http.ResponseWriter, r *http.Request) {
|
||||
s.renderDone(w, "Sign-in failed", "The identity provider rejected the sign-in.")
|
||||
return
|
||||
}
|
||||
var webToken string
|
||||
var webToken, returnURL string
|
||||
if err := s.st.Update(func(st *store.State) error {
|
||||
var dev *store.DeviceAuth
|
||||
for _, d := range st.Devices {
|
||||
@@ -257,6 +257,7 @@ func (s *Server) handleOIDCCallback(w http.ResponseWriter, r *http.Request) {
|
||||
return nil
|
||||
}
|
||||
dev.SessionToken = token
|
||||
returnURL = dev.ReturnURL
|
||||
return nil
|
||||
}); err != nil {
|
||||
s.renderDone(w, "Sign-in failed", err.Error())
|
||||
@@ -267,7 +268,7 @@ func (s *Server) handleOIDCCallback(w http.ResponseWriter, r *http.Request) {
|
||||
http.Redirect(w, r, "/web/vaults", http.StatusFound)
|
||||
return
|
||||
}
|
||||
s.renderDone(w, "Device approved", "You can close this page and return to the application.")
|
||||
s.renderApproved(w, returnURL)
|
||||
}
|
||||
|
||||
// linkOIDCUser finds the account behind an identity, provisioning one when
|
||||
|
||||
@@ -140,7 +140,8 @@ func TestOIDCApprovesAWaitingClient(t *testing.T) {
|
||||
idp := newFakeIdP(t, "subject-2", "dev@example.com", "devuser")
|
||||
h := newHarnessWith(t, idp.withOIDC(true, "everyone"))
|
||||
|
||||
status, start := h.call("POST", "/api/v1/auth/device/start", "", map[string]string{"label": "laptop"})
|
||||
status, start := h.call("POST", "/api/v1/auth/device/start", "",
|
||||
map[string]string{"label": "phone", "return_url": "keeporg://device-approved"})
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("device start: %d %v", status, start)
|
||||
}
|
||||
@@ -151,6 +152,10 @@ func TestOIDCApprovesAWaitingClient(t *testing.T) {
|
||||
if status != http.StatusOK || !strings.Contains(page, "Device approved") {
|
||||
t.Fatalf("device approval through single sign-on: %d\n%s", status, page)
|
||||
}
|
||||
// A phone is handed back to its app from the provider's redirect too.
|
||||
if !strings.Contains(page, `href="keeporg://device-approved"`) {
|
||||
t.Fatalf("no link back to the app after single sign-on:\n%s", page)
|
||||
}
|
||||
// A device approval must not also hand the browser a session cookie.
|
||||
if b.cookie() != "" {
|
||||
t.Fatal("approving a device set a web session cookie")
|
||||
|
||||
@@ -31,8 +31,9 @@ p.sub{margin:0 0 1.25rem;opacity:.7;font-size:.9rem}
|
||||
label{display:block;font-size:.8rem;text-transform:uppercase;letter-spacing:.04em;opacity:.7;margin:.9rem 0 .25rem}
|
||||
input{width:100%;padding:.6rem .7rem;font:inherit;color:var(--fg);background:transparent;
|
||||
border:1px solid var(--line);border-radius:8px}
|
||||
button{width:100%;margin-top:1.4rem;padding:.7rem;font:inherit;font-weight:600;color:#fff;
|
||||
button,a.button{width:100%;margin-top:1.4rem;padding:.7rem;font:inherit;font-weight:600;color:#fff;
|
||||
background:var(--accent);border:0;border-radius:8px;cursor:pointer}
|
||||
a.button{display:block;box-sizing:border-box;text-align:center;text-decoration:none}
|
||||
.err{margin:0 0 1rem;padding:.6rem .7rem;border-radius:8px;background:#f8d7da;color:#842029;font-size:.9rem}
|
||||
.alt{display:block;margin-top:1rem;text-align:center;font-size:.9rem;color:var(--accent)}
|
||||
code{font-size:1.1rem;letter-spacing:.08em}
|
||||
@@ -250,7 +251,8 @@ to ask for.</p>
|
||||
{{end}}`))
|
||||
|
||||
var doneTmpl = template.Must(template.Must(pages.Clone()).Parse(`
|
||||
{{define "body"}}<h1>{{.Title}}</h1><p class="sub">{{.Message}}</p>{{end}}`))
|
||||
{{define "body"}}<h1>{{.Title}}</h1><p class="sub">{{.Message}}</p>
|
||||
{{if .Return}}<p><a class="button" href="{{.Return}}">Return to KeepOrg</a></p>{{end}}{{end}}`))
|
||||
|
||||
func (s *Server) renderDevice(w http.ResponseWriter, code, errMsg string) {
|
||||
render(w, deviceTmpl, map[string]any{
|
||||
@@ -290,6 +292,18 @@ func (s *Server) renderDone(w http.ResponseWriter, title, msg string) {
|
||||
render(w, doneTmpl, map[string]any{"Title": title, "Message": msg})
|
||||
}
|
||||
|
||||
// renderApproved ends a device approval. A client that gave a return URL
|
||||
// gets a link back to itself; html/template would neutralise its custom
|
||||
// scheme, which is why it is marked safe here, after validReturnURL.
|
||||
func (s *Server) renderApproved(w http.ResponseWriter, returnURL string) {
|
||||
d := map[string]any{"Title": "Device approved",
|
||||
"Message": "You can close this page and return to the application."}
|
||||
if returnURL != "" && validReturnURL(returnURL) {
|
||||
d["Return"] = template.URL(returnURL)
|
||||
}
|
||||
render(w, doneTmpl, d)
|
||||
}
|
||||
|
||||
func render(w http.ResponseWriter, t *template.Template, data map[string]any) {
|
||||
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||
w.Header().Set("Referrer-Policy", "no-referrer")
|
||||
|
||||
Reference in New Issue
Block a user