Commit 7c2289e0 authored by Eric Chiang's avatar Eric Chiang

*: rename internally used "state" form value to "req"

"state" means something specific to OAuth2 and SAML so we don't
want to confuse developers who are working on this.

Also don't use "session" which could easily be confused with HTTP
cookies.
parent a3235d02
...@@ -148,11 +148,9 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { ...@@ -148,11 +148,9 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
s.renderError(w, http.StatusInternalServerError, errServerError, "") s.renderError(w, http.StatusInternalServerError, errServerError, "")
return return
} }
state := authReq.ID
if len(s.connectors) == 1 { if len(s.connectors) == 1 {
for id := range s.connectors { for id := range s.connectors {
http.Redirect(w, r, s.absPath("/auth", id)+"?state="+state, http.StatusFound) http.Redirect(w, r, s.absPath("/auth", id)+"?req="+authReq.ID, http.StatusFound)
return return
} }
} }
...@@ -168,7 +166,7 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { ...@@ -168,7 +166,7 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
i++ i++
} }
s.templates.login(w, connectorInfos, state) s.templates.login(w, connectorInfos, authReq.ID)
} }
func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) { func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
...@@ -179,7 +177,7 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) { ...@@ -179,7 +177,7 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
return return
} }
authReqID := r.FormValue("state") authReqID := r.FormValue("req")
// TODO(ericchiang): cache user identity. // TODO(ericchiang): cache user identity.
...@@ -198,6 +196,9 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) { ...@@ -198,6 +196,9 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
switch conn := conn.Connector.(type) { switch conn := conn.Connector.(type) {
case connector.CallbackConnector: case connector.CallbackConnector:
// Use the auth request ID as the "state" token.
//
// TODO(ericchiang): Is this appropriate or should we also be using a nonce?
callbackURL, err := conn.LoginURL(s.absURL("/callback"), authReqID) callbackURL, err := conn.LoginURL(s.absURL("/callback"), authReqID)
if err != nil { if err != nil {
log.Printf("Connector %q returned error when creating callback: %v", connID, err) log.Printf("Connector %q returned error when creating callback: %v", connID, err)
...@@ -342,11 +343,11 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth ...@@ -342,11 +343,11 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
if err := s.storage.UpdateAuthRequest(authReq.ID, updater); err != nil { if err := s.storage.UpdateAuthRequest(authReq.ID, updater); err != nil {
return "", fmt.Errorf("failed to update auth request: %v", err) return "", fmt.Errorf("failed to update auth request: %v", err)
} }
return path.Join(s.issuerURL.Path, "/approval") + "?state=" + authReq.ID, nil return path.Join(s.issuerURL.Path, "/approval") + "?req=" + authReq.ID, nil
} }
func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) { func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
authReq, err := s.storage.GetAuthRequest(r.FormValue("state")) authReq, err := s.storage.GetAuthRequest(r.FormValue("req"))
if err != nil { if err != nil {
log.Printf("Failed to get auth request: %v", err) log.Printf("Failed to get auth request: %v", err)
s.renderError(w, http.StatusInternalServerError, errServerError, "") s.renderError(w, http.StatusInternalServerError, errServerError, "")
......
...@@ -138,29 +138,29 @@ func (n byName) Len() int { return len(n) } ...@@ -138,29 +138,29 @@ func (n byName) Len() int { return len(n) }
func (n byName) Less(i, j int) bool { return n[i].Name < n[j].Name } func (n byName) Less(i, j int) bool { return n[i].Name < n[j].Name }
func (n byName) Swap(i, j int) { n[i], n[j] = n[j], n[i] } func (n byName) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
func (t *templates) login(w http.ResponseWriter, connectors []connectorInfo, state string) { func (t *templates) login(w http.ResponseWriter, connectors []connectorInfo, authReqID string) {
sort.Sort(byName(connectors)) sort.Sort(byName(connectors))
data := struct { data := struct {
TemplateConfig TemplateConfig
Connectors []connectorInfo Connectors []connectorInfo
State string AuthReqID string
}{t.globalData, connectors, state} }{t.globalData, connectors, authReqID}
renderTemplate(w, t.loginTmpl, data) renderTemplate(w, t.loginTmpl, data)
} }
func (t *templates) password(w http.ResponseWriter, state, callback, lastUsername string, lastWasInvalid bool) { func (t *templates) password(w http.ResponseWriter, authReqID, callback, lastUsername string, lastWasInvalid bool) {
data := struct { data := struct {
TemplateConfig TemplateConfig
State string AuthReqID string
PostURL string PostURL string
Username string Username string
Invalid bool Invalid bool
}{t.globalData, state, callback, lastUsername, lastWasInvalid} }{t.globalData, authReqID, callback, lastUsername, lastWasInvalid}
renderTemplate(w, t.passwordTmpl, data) renderTemplate(w, t.passwordTmpl, data)
} }
func (t *templates) approval(w http.ResponseWriter, state, username, clientName string, scopes []string) { func (t *templates) approval(w http.ResponseWriter, authReqID, username, clientName string, scopes []string) {
accesses := []string{} accesses := []string{}
for _, scope := range scopes { for _, scope := range scopes {
access, ok := scopeDescriptions[scope] access, ok := scopeDescriptions[scope]
...@@ -173,9 +173,9 @@ func (t *templates) approval(w http.ResponseWriter, state, username, clientName ...@@ -173,9 +173,9 @@ func (t *templates) approval(w http.ResponseWriter, state, username, clientName
TemplateConfig TemplateConfig
User string User string
Client string Client string
State string AuthReqID string
Scopes []string Scopes []string
}{t.globalData, username, clientName, state, accesses} }{t.globalData, username, clientName, authReqID, accesses}
renderTemplate(w, t.approvalTmpl, data) renderTemplate(w, t.approvalTmpl, data)
} }
......
...@@ -25,7 +25,7 @@ var defaultTemplates = map[string]string{ ...@@ -25,7 +25,7 @@ var defaultTemplates = map[string]string{
<div> <div>
<div class="form-row"> <div class="form-row">
<form method="post"> <form method="post">
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
<input type="hidden" name="approval" value="approve"> <input type="hidden" name="approval" value="approve">
<button type="submit" class="btn btn-success"> <button type="submit" class="btn btn-success">
<span class="btn-text">Grant Access</span> <span class="btn-text">Grant Access</span>
...@@ -34,7 +34,7 @@ var defaultTemplates = map[string]string{ ...@@ -34,7 +34,7 @@ var defaultTemplates = map[string]string{
</div> </div>
<div class="form-row"> <div class="form-row">
<form method="post"> <form method="post">
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
<input type="hidden" name="approval" value="rejected"> <input type="hidden" name="approval" value="rejected">
<button type="submit" class="btn btn-provider"> <button type="submit" class="btn btn-provider">
<span class="btn-text">Cancel</span> <span class="btn-text">Cancel</span>
...@@ -300,7 +300,7 @@ var defaultTemplates = map[string]string{ ...@@ -300,7 +300,7 @@ var defaultTemplates = map[string]string{
<div> <div>
{{ range $c := .Connectors }} {{ range $c := .Connectors }}
<div class="form-row"> <div class="form-row">
<a href="{{ $c.URL }}?state={{ $.State }}" target="_self"> <a href="{{ $c.URL }}?req={{ $.AuthReqID }}" target="_self">
<button class="btn btn-provider"> <button class="btn btn-provider">
<span class="btn-icon btn-icon-{{ $c.ID }}"></span> <span class="btn-icon btn-icon-{{ $c.ID }}"></span>
<span class="btn-text">Log in with {{ $c.Name }}</span> <span class="btn-text">Log in with {{ $c.Name }}</span>
...@@ -344,7 +344,7 @@ var defaultTemplates = map[string]string{ ...@@ -344,7 +344,7 @@ var defaultTemplates = map[string]string{
</div> </div>
<input tabindex="2" required id="password" name="password" type="password" class="input-box" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/> <input tabindex="2" required id="password" name="password" type="password" class="input-box" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/>
</div> </div>
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
{{ if .Invalid }} {{ if .Invalid }}
<div class="error-box"> <div class="error-box">
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
<div> <div>
<div class="form-row"> <div class="form-row">
<form method="post"> <form method="post">
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
<input type="hidden" name="approval" value="approve"> <input type="hidden" name="approval" value="approve">
<button type="submit" class="btn btn-success"> <button type="submit" class="btn btn-success">
<span class="btn-text">Grant Access</span> <span class="btn-text">Grant Access</span>
...@@ -28,7 +28,7 @@ ...@@ -28,7 +28,7 @@
</div> </div>
<div class="form-row"> <div class="form-row">
<form method="post"> <form method="post">
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
<input type="hidden" name="approval" value="rejected"> <input type="hidden" name="approval" value="rejected">
<button type="submit" class="btn btn-provider"> <button type="submit" class="btn btn-provider">
<span class="btn-text">Cancel</span> <span class="btn-text">Cancel</span>
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
<div> <div>
{{ range $c := .Connectors }} {{ range $c := .Connectors }}
<div class="form-row"> <div class="form-row">
<a href="{{ $c.URL }}?state={{ $.State }}" target="_self"> <a href="{{ $c.URL }}?req={{ $.AuthReqID }}" target="_self">
<button class="btn btn-provider"> <button class="btn btn-provider">
<span class="btn-icon btn-icon-{{ $c.ID }}"></span> <span class="btn-icon btn-icon-{{ $c.ID }}"></span>
<span class="btn-text">Log in with {{ $c.Name }}</span> <span class="btn-text">Log in with {{ $c.Name }}</span>
......
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
</div> </div>
<input tabindex="2" required id="password" name="password" type="password" class="input-box" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/> <input tabindex="2" required id="password" name="password" type="password" class="input-box" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/>
</div> </div>
<input type="hidden" name="state" value="{{ .State }}"/> <input type="hidden" name="req" value="{{ .AuthReqID }}"/>
{{ if .Invalid }} {{ if .Invalid }}
<div class="error-box"> <div class="error-box">
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment