Skip to content

Commit 37d88e8

Browse files
danielgtaylordanielgtaylor
authored
Merge pull request #335 from rest-sh/oauth-callback-pages
feat(auth): theme and customize OAuth callback pages
2 parents 325daa2 + a1c4443 commit 37d88e8

11 files changed

Lines changed: 541 additions & 42 deletions

File tree

‎internal/auth/oauth_authcode.go‎

Lines changed: 298 additions & 27 deletions
Large diffs are not rendered by default.

‎internal/auth/oauth_authcode_test.go‎

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ func TestAuthCode_CallbackPageReflectsTokenExchangeResult(t *testing.T) {
704704
if err != nil {
705705
t.Fatalf("OnRequest: %v", err)
706706
}
707-
if body := <-bodyCh; !strings.Contains(body, "Authorization code received") || !strings.Contains(body, "Authentication successful") {
707+
if body := <-bodyCh; !strings.Contains(body, "Login Successful!") || !strings.Contains(body, `class="check"`) || !strings.Contains(body, "@keyframes success-bg") {
708708
t.Fatalf("callback body = %q", body)
709709
}
710710
})
@@ -740,12 +740,130 @@ func TestAuthCode_CallbackPageReflectsTokenExchangeResult(t *testing.T) {
740740
if err == nil {
741741
t.Fatal("expected token exchange error")
742742
}
743-
if body := <-bodyCh; !strings.Contains(body, "Authorization code received") || !strings.Contains(body, "Authentication failed") {
743+
if body := <-bodyCh; !strings.Contains(body, "Authentication failed") || !strings.Contains(body, `class="x"`) || !strings.Contains(body, "@keyframes failure-bg") {
744744
t.Fatalf("callback body = %q", body)
745745
}
746746
})
747747
}
748748

749+
func TestOAuthCallbackErrorPageEscapesDetail(t *testing.T) {
750+
body := oauthCallbackErrorPage("Authentication failed", `<script>alert("nope")</script>`, "")
751+
if strings.Contains(body, "<script>") {
752+
t.Fatalf("callback body includes raw script: %q", body)
753+
}
754+
if !strings.Contains(body, `&lt;script&gt;alert(&#34;nope&#34;)&lt;/script&gt;`) {
755+
t.Fatalf("callback body does not include escaped detail: %q", body)
756+
}
757+
}
758+
759+
func TestOAuthCallbackPageUsesConfiguredBackgroundColor(t *testing.T) {
760+
body := oauthCallbackSuccessPage("Login Successful!", "Done.", "#50fa7b")
761+
if !strings.Contains(body, "to { background: #50fa7b; }") {
762+
t.Fatalf("callback body does not use configured color: %q", body)
763+
}
764+
}
765+
766+
func TestOAuthCallbackPageRejectsInvalidBackgroundColor(t *testing.T) {
767+
body := oauthCallbackErrorPage("Authentication failed", "Nope.", `red; background: url("bad")`)
768+
if strings.Contains(body, "url(") {
769+
t.Fatalf("callback body includes invalid CSS color: %q", body)
770+
}
771+
if !strings.Contains(body, "to { background: #E94F37; }") {
772+
t.Fatalf("callback body did not fall back to default failure color: %q", body)
773+
}
774+
}
775+
776+
func TestOAuthCallbackPagesUseCustomHTMLFields(t *testing.T) {
777+
h := &AuthorizationCode{
778+
CallbackSuccessHTML: `<html><body><h1>Welcome to my-tool</h1></body></html>`,
779+
CallbackErrorHTML: `<html><body><h1>$ERROR</h1><p>$DETAILS</p></body></html>`,
780+
}
781+
if got, want := h.oauthCallbackSuccessPage("Login Successful!", "Done."), h.CallbackSuccessHTML; got != want {
782+
t.Fatalf("custom success body = %q, want %q", got, want)
783+
}
784+
got := h.oauthCallbackErrorPage("Authentication failed", `<script>alert("nope")</script>`)
785+
want := `<html><body><h1>Authentication failed</h1><p>&lt;script&gt;alert(&#34;nope&#34;)&lt;/script&gt;</p></body></html>`
786+
if got != want {
787+
t.Fatalf("custom error body = %q, want %q", got, want)
788+
}
789+
}
790+
791+
func TestOAuthCallbackPagesUseCustomHTMLParams(t *testing.T) {
792+
h := &AuthorizationCode{
793+
CallbackSuccessHTML: `<html>field success</html>`,
794+
CallbackErrorHTML: `<html>field error</html>`,
795+
}
796+
pages := h.oauthCallbackPages(map[string]string{
797+
callbackSuccessHTMLParam: `<html><body><h1>$TITLE</h1><p>$DETAILS</p></body></html>`,
798+
callbackErrorHTMLParam: `<html><body><h1>$ERROR</h1><p>$DETAILS</p></body></html>`,
799+
})
800+
if got, want := pages.successPage("Login Successful!", "Done."), `<html><body><h1>Login Successful!</h1><p>Done.</p></body></html>`; got != want {
801+
t.Fatalf("custom success body = %q, want %q", got, want)
802+
}
803+
got := pages.errorPage("Error: access_denied", `bad <reason>`, "access_denied")
804+
want := `<html><body><h1>access_denied</h1><p>bad &lt;reason&gt;</p></body></html>`
805+
if got != want {
806+
t.Fatalf("custom error body = %q, want %q", got, want)
807+
}
808+
}
809+
810+
func TestAuthCode_CustomCallbackHTMLParamsAreNotForwarded(t *testing.T) {
811+
h := &AuthorizationCode{
812+
HTTPClient: testHTTPClient(func(r *http.Request) (*http.Response, error) {
813+
if err := r.ParseForm(); err != nil {
814+
t.Fatalf("ParseForm: %v", err)
815+
}
816+
if got := r.FormValue(callbackSuccessHTMLParam); got != "" {
817+
t.Fatalf("%s forwarded to token endpoint: %q", callbackSuccessHTMLParam, got)
818+
}
819+
if got := r.FormValue(callbackErrorHTMLParam); got != "" {
820+
t.Fatalf("%s forwarded to token endpoint: %q", callbackErrorHTMLParam, got)
821+
}
822+
return testResponse(200, "application/json", `{"access_token":"custom-html-token","token_type":"bearer","expires_in":3600}`), nil
823+
}),
824+
OpenBrowser: func(raw string) error {
825+
go func() {
826+
authorizeURL, err := url.Parse(raw)
827+
if err != nil {
828+
t.Errorf("parse authorize URL: %v", err)
829+
return
830+
}
831+
if got := authorizeURL.Query().Get(callbackSuccessHTMLParam); got != "" {
832+
t.Errorf("%s forwarded to authorize endpoint: %q", callbackSuccessHTMLParam, got)
833+
return
834+
}
835+
if got := authorizeURL.Query().Get(callbackErrorHTMLParam); got != "" {
836+
t.Errorf("%s forwarded to authorize endpoint: %q", callbackErrorHTMLParam, got)
837+
return
838+
}
839+
callbackURL, state := mustCallbackURL(t, raw)
840+
resp, err := http.Get(fmt.Sprintf("%s/?state=%s&code=good-code", callbackURL, url.QueryEscape(state)))
841+
if err != nil {
842+
t.Errorf("callback request failed: %v", err)
843+
return
844+
}
845+
resp.Body.Close()
846+
}()
847+
return nil
848+
},
849+
}
850+
req, _ := http.NewRequest("GET", "https://api.example.com", nil)
851+
err := h.OnRequest(req, map[string]string{
852+
"client_id": "id1",
853+
"authorize_url": "https://auth.example.com/authorize",
854+
"token_url": "https://auth.example.com/token",
855+
"redirect_port": availablePort(t),
856+
callbackSuccessHTMLParam: `<html>success</html>`,
857+
callbackErrorHTMLParam: `<html>error</html>`,
858+
})
859+
if err != nil {
860+
t.Fatalf("OnRequest: %v", err)
861+
}
862+
if got := req.Header.Get("Authorization"); got != "Bearer custom-html-token" {
863+
t.Fatalf("Authorization = %q, want custom HTML token", got)
864+
}
865+
}
866+
749867
func TestAuthCode_ManualCodeFallback(t *testing.T) {
750868
var stderr bytes.Buffer
751869
h := &AuthorizationCode{

‎internal/auth/oauth_common.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,12 @@ func applyTokenAuthHeader(req *http.Request, params map[string]string) {
451451

452452
func applyOAuthTokenExtraParams(form url.Values, params map[string]string) {
453453
for key, value := range extraOAuthParams(params, map[string]bool{
454-
"_cache_key": true,
455-
"authorize_url": true,
456-
"cache_key": true,
457-
"issuer_url": true,
454+
"_cache_key": true,
455+
"authorize_url": true,
456+
"cache_key": true,
457+
callbackErrorHTMLParam: true,
458+
callbackSuccessHTMLParam: true,
459+
"issuer_url": true,
458460
// TODO(openapi-3.2): use oauth2_metadata_url for RFC 8414 metadata
459461
// discovery in place of, or alongside, issuer_url.
460462
"oauth2_metadata_url": true,

‎internal/auth/oauth_common_test.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,20 @@ func TestApplyTokenAuthHeaderPercentEncodesBasicCredentials(t *testing.T) {
272272
func TestApplyOAuthTokenExtraParamsOmitsMetadataURL(t *testing.T) {
273273
form := url.Values{}
274274
applyOAuthTokenExtraParams(form, map[string]string{
275-
"audience": "https://api.example.com/",
276-
"oauth2_metadata_url": "https://auth.example.com/.well-known/oauth-authorization-server",
275+
"audience": "https://api.example.com/",
276+
callbackErrorHTMLParam: "<html>error</html>",
277+
callbackSuccessHTMLParam: "<html>success</html>",
278+
"oauth2_metadata_url": "https://auth.example.com/.well-known/oauth-authorization-server",
277279
})
278280
if got := form.Get("audience"); got != "https://api.example.com/" {
279281
t.Fatalf("audience = %q", got)
280282
}
283+
if got := form.Get(callbackSuccessHTMLParam); got != "" {
284+
t.Fatalf("%s should not be forwarded, got %q", callbackSuccessHTMLParam, got)
285+
}
286+
if got := form.Get(callbackErrorHTMLParam); got != "" {
287+
t.Fatalf("%s should not be forwarded, got %q", callbackErrorHTMLParam, got)
288+
}
281289
if got := form.Get("oauth2_metadata_url"); got != "" {
282290
t.Fatalf("oauth2_metadata_url should not be forwarded, got %q", got)
283291
}

‎internal/auth/oauth_device_code.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,8 @@ func (h *DeviceCode) requestDeviceAuthorization(ctx context.Context, params map[
248248
"_cache_key": true,
249249
"authorize_url": true,
250250
"cache_key": true,
251+
callbackErrorHTMLParam: true,
252+
callbackSuccessHTMLParam: true,
251253
"device_authorization_url": true,
252254
"issuer_url": true,
253255
"redirect_port": true,

‎internal/cli/auth.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,14 @@ func (c *CLI) authHandlerFor(ac *config.AuthConfig, opts authHandlerOptions) (au
8282
}, nil
8383
case "oauth-authorization-code":
8484
return &auth.AuthorizationCode{
85-
Cache: auth.NewTokenCache(c.tokenCachePath()),
86-
HTTPClient: &http.Client{Transport: c.baseHTTPTransport()},
87-
Stderr: c.Stderr,
88-
CanPrompt: c.canPromptCode(),
89-
NoBrowser: opts.NoBrowser,
90-
Verbose: opts.Verbose,
85+
Cache: auth.NewTokenCache(c.tokenCachePath()),
86+
HTTPClient: &http.Client{Transport: c.baseHTTPTransport()},
87+
Stderr: c.Stderr,
88+
CanPrompt: c.canPromptCode(),
89+
NoBrowser: opts.NoBrowser,
90+
Verbose: opts.Verbose,
91+
CallbackSuccessColor: output.ThemeTokenColor("status_2xx"),
92+
CallbackFailureColor: output.ThemeTokenColor("status_error"),
9193
}, nil
9294
case "oauth-device-code":
9395
return &auth.DeviceCode{

‎internal/cli/auth_internal_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import (
77
"strings"
88
"testing"
99

10+
"github.com/rest-sh/restish/v2/internal/auth"
1011
"github.com/rest-sh/restish/v2/internal/config"
12+
"github.com/rest-sh/restish/v2/internal/output"
1113
"github.com/rest-sh/restish/v2/internal/spec"
1214
)
1315

@@ -97,3 +99,33 @@ func TestConfiguredCredentialsCountsAuthRef(t *testing.T) {
9799
t.Fatalf("empty credential counted as configured: %#v", got)
98100
}
99101
}
102+
103+
func TestAuthHandlerForOAuthUsesThemeCallbackColors(t *testing.T) {
104+
if err := output.SetTheme(output.ThemeEntries{
105+
"status_2xx": "bold #00ff00",
106+
"status_error": "italic #ff0000",
107+
}); err != nil {
108+
t.Fatalf("SetTheme: %v", err)
109+
}
110+
t.Cleanup(func() {
111+
if err := output.SetTheme(nil); err != nil {
112+
t.Fatalf("reset theme: %v", err)
113+
}
114+
})
115+
116+
c := New()
117+
handler, err := c.authHandlerFor(&config.AuthConfig{Type: "oauth-authorization-code"}, authHandlerOptions{})
118+
if err != nil {
119+
t.Fatalf("authHandlerFor: %v", err)
120+
}
121+
oauthHandler, ok := handler.(*auth.AuthorizationCode)
122+
if !ok {
123+
t.Fatalf("handler = %T, want *auth.AuthorizationCode", handler)
124+
}
125+
if oauthHandler.CallbackSuccessColor != "#00ff00" {
126+
t.Fatalf("success color = %q, want #00ff00", oauthHandler.CallbackSuccessColor)
127+
}
128+
if oauthHandler.CallbackFailureColor != "#ff0000" {
129+
t.Fatalf("failure color = %q, want #ff0000", oauthHandler.CallbackFailureColor)
130+
}
131+
}

‎internal/output/style.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,20 @@ func StyleText(tokenName, text string) string {
222222
return out.String()
223223
}
224224

225+
// ThemeTokenColor returns the active theme's foreground color for tokenName.
226+
// It returns an empty string when the token is unknown or has no color.
227+
func ThemeTokenColor(tokenName string) string {
228+
token, err := themeTokenType(tokenName)
229+
if err != nil {
230+
return ""
231+
}
232+
entry := activeStyle().Get(token)
233+
if !entry.Colour.IsSet() {
234+
return ""
235+
}
236+
return entry.Colour.String()
237+
}
238+
225239
func activeStyle() *chroma.Style {
226240
themeStateMu.RLock()
227241
style := restishStyle

‎internal/output/style_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,27 @@ func TestBuildThemeTextToken(t *testing.T) {
6161
}
6262
}
6363

64+
func TestThemeTokenColorUsesActiveTheme(t *testing.T) {
65+
if err := SetTheme(ThemeEntries{"status_2xx": "bold #00ff00", "status_error": "italic #ff0000"}); err != nil {
66+
t.Fatalf("SetTheme: %v", err)
67+
}
68+
t.Cleanup(func() {
69+
if err := SetTheme(nil); err != nil {
70+
t.Fatalf("reset theme: %v", err)
71+
}
72+
})
73+
74+
if got, want := ThemeTokenColor("status_2xx"), "#00ff00"; got != want {
75+
t.Fatalf("status_2xx color = %q, want %q", got, want)
76+
}
77+
if got, want := ThemeTokenColor("status_error"), "#ff0000"; got != want {
78+
t.Fatalf("status_error color = %q, want %q", got, want)
79+
}
80+
if got := ThemeTokenColor("not_a_token"); got != "" {
81+
t.Fatalf("unknown token color = %q, want empty", got)
82+
}
83+
}
84+
6485
func TestBuildThemeDefaultHeaderKeyCanDifferFromKey(t *testing.T) {
6586
style, err := BuildTheme(nil)
6687
if err != nil {

‎site/content/en/docs/guides/oauth.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,27 @@ Some providers distinguish `localhost` from `127.0.0.1`. Restish sends
138138
`localhost` in the authorization request, so exact-match providers must allow
139139
the `localhost` URL.
140140

141+
The browser callback page uses the active Restish theme. To brand that local
142+
page, set `callback_success_html` and/or `callback_error_html` on the
143+
authorization-code profile:
144+
145+
```jsonc
146+
{
147+
"type": "oauth-authorization-code",
148+
"params": {
149+
"authorize_url": "https://issuer.test/authorize",
150+
"token_url": "https://issuer.test/oauth/token",
151+
"client_id": "env:CLIENT_ID",
152+
"callback_success_html": "<html><body><h1>Signed in</h1><p>You can return to the terminal.</p></body></html>",
153+
"callback_error_html": "<html><body><h1>Sign-in failed: $ERROR</h1><p>$DETAILS</p></body></html>"
154+
}
155+
}
156+
```
157+
158+
Callback HTML supports `$TITLE` and `$DETAILS`; failure HTML also supports
159+
`$ERROR`. Restish escapes substituted values before inserting them, and does
160+
not forward callback HTML params to OAuth authorization or token endpoints.
161+
141162
Use `--rsh-no-browser` when browser launch is not possible:
142163

143164
```bash

0 commit comments

Comments
 (0)