diff --git a/admin/admin.go b/admin/admin.go index f578c520..f9b5cbdd 100644 --- a/admin/admin.go +++ b/admin/admin.go @@ -14,12 +14,14 @@ import ( "log" "net/http" "net/url" + "os" "path/filepath" "regexp" "strconv" "strings" "github.com/gin-gonic/gin" + "github.com/joho/godotenv" "gorm.io/gorm" ) @@ -723,6 +725,9 @@ func (a *Admin) AdminSettings(c *gin.Context) { return } settings := a.b.GetSettings() + // The install wizard appends client_id to .env without touching the + // environment; load it so a site installed that way shows as configured. + _ = godotenv.Load(datadir.Path(".env")) c.HTML(http.StatusOK, "admin_settings.html", gin.H{ "posts": a.b.GetPosts(true), "logged_in": a.auth.IsLoggedIn(c), @@ -734,6 +739,11 @@ func (a *Admin) AdminSettings(c *gin.Context) { "setting_groups": GroupSettings(settings), "nav_pages": a.b.GetNavPages(), "themes": ListThemes(), + + // The Login methods section (#668). + "github_client_id": os.Getenv("client_id"), + "github_callback_url": blog.RequestOrigin(c) + "/login", + "email_login_enabled": a.auth.EmailLoginEnabled(), }) } diff --git a/admin/login_methods.go b/admin/login_methods.go new file mode 100644 index 00000000..fed174f6 --- /dev/null +++ b/admin/login_methods.go @@ -0,0 +1,71 @@ +package admin + +import ( + "errors" + "io/fs" + "net/http" + "os" + "regexp" + "strings" + + "github.com/gin-gonic/gin" + + "goblog/datadir" +) + +// githubCredential matches a GitHub OAuth app's client id ("Ov23li…", +// "Iv1.…") or secret (hex). Anything else is refused rather than quoted, so a +// value can never add a line to .env. +var githubCredential = regexp.MustCompile(`^[A-Za-z0-9._-]+$`) + +// SaveGithubLogin (PUT /api/v1/login-methods/github) stores a GitHub OAuth +// app's client id and secret in .env and in the environment, which the login +// page and /login/github read on each request, so GitHub login works without +// a restart. Before this only the install wizard could set them, so a site +// installed with a password admin needed .env edited by hand (#668). +func (a *Admin) SaveGithubLogin(c *gin.Context) { + if !a.auth.IsAdmin(c) { + c.JSON(http.StatusUnauthorized, "Not Authorized") + return + } + var req struct { + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + } + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, "Couldn't read the request: "+err.Error()) + return + } + id, secret := strings.TrimSpace(req.ClientID), strings.TrimSpace(req.ClientSecret) + if !githubCredential.MatchString(id) || !githubCredential.MatchString(secret) { + c.JSON(http.StatusBadRequest, "Enter the Client ID and a Client Secret exactly as GitHub shows them") + return + } + if err := setGithubEnv(datadir.Path(".env"), id, secret); err != nil { + c.JSON(http.StatusInternalServerError, "Couldn't write the .env file: "+err.Error()) + return + } + os.Setenv("client_id", id) + os.Setenv("client_secret", secret) + c.JSON(http.StatusOK, "GitHub login saved") +} + +// setGithubEnv rewrites the .env file at path with client_id and +// client_secret set to id and secret, keeping every other line. +func setGithubEnv(path, id, secret string) error { + existing, err := os.ReadFile(path) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + var out strings.Builder + if kept := strings.TrimRight(string(existing), "\n"); kept != "" { + for _, line := range strings.Split(kept, "\n") { + key, _, _ := strings.Cut(strings.TrimSpace(line), "=") + if key = strings.TrimSpace(key); key != "client_id" && key != "client_secret" { + out.WriteString(line + "\n") + } + } + } + out.WriteString("client_id=" + id + "\nclient_secret=" + secret + "\n") + return os.WriteFile(path, []byte(out.String()), 0600) +} diff --git a/admin/login_methods_test.go b/admin/login_methods_test.go new file mode 100644 index 00000000..45eb2b0a --- /dev/null +++ b/admin/login_methods_test.go @@ -0,0 +1,70 @@ +package admin_test + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "goblog/admin" + "goblog/blog" + "goblog/datadir" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/mock" + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +// TestSaveGithubLogin checks the Login methods section's save (#668): only +// an admin may call it, a value that could add a line to .env is refused, and +// a good one replaces any earlier client_id/client_secret in .env, keeps the +// other lines, and takes effect in the environment without a restart. +func TestSaveGithubLogin(t *testing.T) { + dir := t.TempDir() + t.Setenv(datadir.EnvVar, dir) + t.Setenv("client_id", "") + t.Setenv("client_secret", "") + env := filepath.Join(dir, ".env") + os.WriteFile(env, []byte("SESSION_KEY=k\nclient_id=old\n# note\nclient_secret=old\ndatabase=sqlite\n"), 0600) + + put := func(isAdmin bool, body string) *httptest.ResponseRecorder { + db, _ := gorm.Open(sqlite.Open(":memory:")) + a := &Auth{} + a.On("IsAdmin", mock.Anything).Return(isAdmin) + b := blog.New(db, a, "test") + ad := admin.New(db, a, &b, "test") + router := gin.New() + router.PUT("/api/v1/login-methods/github", ad.SaveGithubLogin) + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPut, "/api/v1/login-methods/github", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + return w + } + + if w := put(false, `{"client_id":"Ov23li","client_secret":"abc"}`); w.Code != http.StatusUnauthorized { + t.Errorf("non-admin: %d, want 401", w.Code) + } + for _, body := range []string{ + `{"client_id":"Ov23li","client_secret":""}`, + `{"client_id":"Ov23li\nadmin_login=evil","client_secret":"abc"}`, + } { + if w := put(true, body); w.Code != http.StatusBadRequest { + t.Errorf("%s: %d, want 400", body, w.Code) + } + } + if w := put(true, `{"client_id":" Ov23li.x ","client_secret":"abc123"}`); w.Code != http.StatusOK { + t.Fatalf("save: %d %s", w.Code, w.Body.String()) + } + got, _ := os.ReadFile(env) + want := "SESSION_KEY=k\n# note\ndatabase=sqlite\nclient_id=Ov23li.x\nclient_secret=abc123\n" + if string(got) != want { + t.Errorf(".env =\n%s\nwant\n%s", got, want) + } + if os.Getenv("client_id") != "Ov23li.x" || os.Getenv("client_secret") != "abc123" { + t.Errorf("environment not updated: %q %q", os.Getenv("client_id"), os.Getenv("client_secret")) + } +} diff --git a/auth/auth.go b/auth/auth.go index 22e66a14..cb192e47 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -235,8 +235,9 @@ func (a *Auth) UpsertUser(user *BlogUser) (*BlogUser, error) { // completeGithubLogin exchanges an authorization code for a token, stores the // user behind it and puts the token in the session. GithubCallback is its only // caller: the code used to arrive by POST from the login page, which meant -// anyone could hand the server a code for any account (#637). -func (a *Auth) completeGithubLogin(c *gin.Context, code string) error { +// anyone could hand the server a code for any account (#637). promote makes +// the GitHub user an admin; GithubCallback decides it. +func (a *Auth) completeGithubLogin(c *gin.Context, code string, promote bool) error { data, err := a.requestAccessToken(code) if err != nil { return errors.New("error requesting token access: " + err.Error()) @@ -263,6 +264,11 @@ func (a *Auth) completeGithubLogin(c *gin.Context, code string) error { } else if err != nil { log.Println("Error ensuring admin user: " + err.Error()) } + if promote { + if err := a.PromoteAdmin(stored.ID); err != nil { + return errors.New("error making the GitHub user an admin: " + err.Error()) + } + } // Set but do not save: GithubCallback saves once, so the response carries // a single Set-Cookie holding both this token and the cleared OAuth state. diff --git a/auth/github_oauth.go b/auth/github_oauth.go index 2a4f18e2..718be79a 100644 --- a/auth/github_oauth.go +++ b/auth/github_oauth.go @@ -28,6 +28,10 @@ const ( // the flow; this package reads them when GitHub returns. OAuthStateKey = "oauth_state" OAuthNextKey = "oauth_next" + // OAuthAdminKey asks GithubCallback to make the GitHub account that + // comes back an admin, if the browser was signed in as an admin when it + // returned: how a password admin moves to GitHub login (#668). + OAuthAdminKey = "oauth_admin" ) // NewOAuthState returns an unguessable value to tie an authorize request to @@ -51,12 +55,15 @@ func (a *Auth) GithubCallback(c *gin.Context) { want, _ := session.Get(OAuthStateKey).(string) next, _ := session.Get(OAuthNextKey).(string) + // Asked before completeGithubLogin replaces the session's user. + promote := session.Get(OAuthAdminKey) == true && a.IsAdmin(c) // One shot: whether or not it matches, this state is spent. Every path // below saves, so a rejected callback still clears it and the same code // and state cannot be presented twice. session.Delete(OAuthStateKey) session.Delete(OAuthNextKey) + session.Delete(OAuthAdminKey) reject := func(reason string) { log.Println("OAuth callback: " + reason) @@ -112,7 +119,7 @@ func (a *Auth) GithubCallback(c *gin.Context) { // completeGithubLogin puts the token on the session; the single save // below persists it together with the cleared state. - if err := a.completeGithubLogin(c, code); err != nil { + if err := a.completeGithubLogin(c, code, promote); err != nil { reject(err.Error()) return } diff --git a/auth/github_oauth_internal_test.go b/auth/github_oauth_internal_test.go index e7d064bb..60715e1a 100644 --- a/auth/github_oauth_internal_test.go +++ b/auth/github_oauth_internal_test.go @@ -75,12 +75,27 @@ func callbackRouter(t *testing.T) (*gin.Engine, *fakeGithub) { s := sessions.Default(c) s.Set(OAuthStateKey, c.Query("state")) s.Set(OAuthNextKey, "/admin/settings") + // ?admin= seeds a password admin and asks for the promotion that + // "Sign in with GitHub as admin" asks for: "self" signs this browser + // in as that admin, "other" leaves it signed out. + if who := c.Query("admin"); who != "" { + pw := BlogUser{Provider: ProviderPassword, ProviderID: "me@example.invalid", AccessToken: "pw-token"} + db.Create(&pw) + db.Create(&AdminUser{BlogUserID: pw.ID}) + if who == "self" { + s.Set("token", pw.AccessToken) + } + s.Set(OAuthAdminKey, true) + } if err := s.Save(); err != nil { t.Fatalf("seeding the session: %v", err) } c.Status(http.StatusOK) }) r.GET("/login", a.GithubCallback) + r.GET("/isadmin", func(c *gin.Context) { + c.String(http.StatusOK, "%v", a.IsAdmin(c)) + }) r.GET("/whoami", func(c *gin.Context) { token, _ := sessions.Default(c).Get("token").(string) c.String(http.StatusOK, token) @@ -277,3 +292,22 @@ func TestGithubCallback_RestartCannotLoop(t *testing.T) { t.Errorf("exchanged anyway (%d calls)", fake.tokenCalls) } } + +// TestGithubCallback_AdminPromotesTheirGithubAccount: "Sign in with GitHub as +// admin" on the Settings page makes the GitHub account that comes back an +// admin, so a password admin can move to GitHub login in one step (#668). +// The request alone is not enough: the browser must be signed in as an admin +// when GitHub returns, or a link could promote anyone. +func TestGithubCallback_AdminPromotesTheirGithubAccount(t *testing.T) { + for who, want := range map[string]string{"self": "true", "other": "false"} { + r, _ := callbackRouter(t) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/seed?state=s&admin="+who, nil)) + + w = get(t, r, "/login?code=good-code&state=s", w.Result().Cookies()) + + if got := get(t, r, "/isadmin", w.Result().Cookies()).Body.String(); got != want { + t.Errorf("admin=%s: GitHub user is admin = %s, want %s", who, got, want) + } + } +} diff --git a/blog/blog.go b/blog/blog.go index eb76cfbe..56e02a2b 100644 --- a/blog/blog.go +++ b/blog/blog.go @@ -1498,6 +1498,7 @@ func (b *Blog) GithubLogin(c *gin.Context) { session := sessions.Default(c) session.Set(auth.OAuthStateKey, state) session.Set(auth.OAuthNextKey, SafeNext(c.Query("next"))) + session.Set(auth.OAuthAdminKey, c.Query("admin") != "") if err := session.Save(); err != nil { // Without a stored state the callback would reject the login, so // there is no point sending them to GitHub. diff --git a/goblog.go b/goblog.go index edb55c79..e180f453 100644 --- a/goblog.go +++ b/goblog.go @@ -573,6 +573,7 @@ func main() { router.POST("/api/v1/preview", goblog._admin.Preview) router.PATCH("/api/v1/settings", goblog._admin.UpdateSettings) router.PATCH("/api/v1/plugin-settings", goblog._admin.UpdatePluginSettings) + router.PUT("/api/v1/login-methods/github", goblog._admin.SaveGithubLogin) router.GET("/api/v1/plugins/status", goblog._admin.PluginStatus) router.GET("/api/v1/plugins/directory", goblog._admin.PluginDirectory) router.POST("/api/v1/plugins/install", goblog._admin.InstallPlugin) diff --git a/themes/default/templates/admin_settings.html b/themes/default/templates/admin_settings.html index 54f61de8..e9cbf362 100644 --- a/themes/default/templates/admin_settings.html +++ b/themes/default/templates/admin_settings.html @@ -59,6 +59,36 @@
+ {{ if .github_client_id }}On Client ID {{ .github_client_id }}.
+ {{ else }}Off{{ end }}
+ Create an OAuth app at github.com/settings/developers
+ with this Authorization callback URL: {{ .github_callback_url }}
+
+ {{ if .email_login_enabled }}On SMTP is configured, so visitors can sign in with a code sent by email.
+ {{ else }}Off Set smtp_host and the other smtp_ settings in .env and restart to let visitors sign in with a code sent by email.{{ end }}
+