Skip to content

Commit

Permalink
idp/openid: fix registration step.
Browse files Browse the repository at this point in the history
If an OpenID Connect login requires the user to register then the
login attempt would fail. The login process now makes sure that all
"candid-login" cookies are created with the same path. Also the
registration template ensures the state parameter is echoed back.
  • Loading branch information
mhilton committed May 18, 2020
1 parent 34a1f99 commit 99d8505
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 7 deletions.
4 changes: 4 additions & 0 deletions idp/idputil/idputil.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ func NameWithDomain(name, domain string) string {
// whilst a login is being processed.
const LoginCookieName = "candid-login"

// LoginCookiePath is the path to associate with the cookie storing the
// current login state.
const LoginCookiePath = "/login"

// LoginState holds the state of the current loging process.
type LoginState struct {
// ReturnTo holds the address to return to after the login has
Expand Down
3 changes: 2 additions & 1 deletion idp/idputil/secret/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (c *Codec) decrypt(out, in []byte) ([]byte, error) {
// SetCookie encodes the given value as a session cookie with the given
// name. The returned value is used the verify the cookie later - it
// should be passed to Cookie when the cookie is retrieved.
func (c *Codec) SetCookie(w http.ResponseWriter, name string, v interface{}) (string, error) {
func (c *Codec) SetCookie(w http.ResponseWriter, name, path string, v interface{}) (string, error) {
out, err := c.encode(v)
if err != nil {
return "", errgo.Mask(err)
Expand All @@ -127,6 +127,7 @@ func (c *Codec) SetCookie(w http.ResponseWriter, name string, v interface{}) (st
http.SetCookie(w, &http.Cookie{
Name: name,
Value: base64.URLEncoding.EncodeToString(out),
Path: path,
})
return base64.RawURLEncoding.EncodeToString(hash[:]), nil
}
Expand Down
6 changes: 3 additions & 3 deletions idp/idputil/secret/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestCookieRoundTrip(t *testing.T) {
}
a.A = 1
a.B = "test"
verification, err := codec.SetCookie(w, "test-cookie", a)
verification, err := codec.SetCookie(w, "test-cookie", "/", a)
c.Assert(err, qt.IsNil)
resp := w.Result()
defer resp.Body.Close()
Expand Down Expand Up @@ -180,7 +180,7 @@ func TestCookieDecodeError(t *testing.T) {
}
a.A = 1
a.B = "test"
_, err := codec.SetCookie(w, "test-cookie", a)
_, err := codec.SetCookie(w, "test-cookie", "/", a)
c.Assert(err, qt.IsNil)
resp := w.Result()
defer resp.Body.Close()
Expand All @@ -206,7 +206,7 @@ func TestCookieValidationError(t *testing.T) {
}
a.A = 1
a.B = "test"
_, err := codec.SetCookie(w, "test-cookie", a)
_, err := codec.SetCookie(w, "test-cookie", "/", a)
c.Assert(err, qt.IsNil)
resp := w.Result()
defer resp.Body.Close()
Expand Down
2 changes: 1 addition & 1 deletion idp/openid/openid-connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (idp *openidConnectIdentityProvider) callback(ctx context.Context, w http.R
return errgo.Mask(err)
}
ls.ProviderID = user.ProviderID
state, err := idp.initParams.Codec.SetCookie(w, idputil.LoginCookieName, ls)
state, err := idp.initParams.Codec.SetCookie(w, idputil.LoginCookieName, idputil.LoginCookiePath, ls)
if err != nil {
return errgo.Mask(err)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/discharger/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (h *handler) Login(p httprequest.Params, req *loginRequest) error {
// Store the requested discharge ID in a session cookie so that
// when the redirect comes back to login-complete we know the
// login was initiated in this session.
state, err := h.params.codec.SetCookie(p.Response, waitCookieName, waitState{
state, err := h.params.codec.SetCookie(p.Response, waitCookieName, "/login-complete", waitState{
DischargeID: req.DischargeID,
})
if err != nil {
Expand Down Expand Up @@ -113,7 +113,7 @@ type redirectLoginRequest struct {
// identity provider which the user must then choose to start the login
// process.
func (h *handler) RedirectLogin(p httprequest.Params, req *redirectLoginRequest) error {
state, err := h.params.codec.SetCookie(p.Response, idputil.LoginCookieName, idputil.LoginState{
state, err := h.params.codec.SetCookie(p.Response, idputil.LoginCookieName, idputil.LoginCookiePath, idputil.LoginState{
ReturnTo: req.ReturnTo,
State: req.State,
Expires: time.Now().Add(15 * time.Minute),
Expand Down
1 change: 1 addition & 0 deletions templates/register
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
</div>
{{end}}
<form class="p-form" method="post" action="register">
<input type="hidden" name="state" value="{{.State}}">
<label for="username">Username</label>
<input type="text" id="username" name="username" class="js_username_input" autocomplete="off">
<p class="p-form-help-text"><span class="js_username_output"></span>@{{.Domain}}</p>
Expand Down

0 comments on commit 99d8505

Please sign in to comment.