cmd/principal,services/identity: Propagate cancellation and errors
from identity server to principal tool.
This change does the following:
(1) Clicking the cancel button at the addcaveats page sends a cancel
error to the identity server.
(2) All errors encountered after a toolRedirectURL is decoded are
propagated to that url.
(3) If the principal tool recieves a identity server error, it will
not attempt to seek a blessings.
This ensure that cancelling a auth flow doesn't leave the principal
tool hanging, and clearly tells the user that they were successful
in their cancellation.
address vanadium/issues#492
Change-Id: Ia142161bcd85bf5d84971e64d7d645ff98d49bfb
diff --git a/cmd/principal/bless.go b/cmd/principal/bless.go
index 80db361..ee0ef1a 100644
--- a/cmd/principal/bless.go
+++ b/cmd/principal/bless.go
@@ -96,16 +96,25 @@
}
}()
- toolState := r.FormValue("state")
- if toolState != state {
+ defer close(result)
+ if r.FormValue("state") != state {
tmplArgs.ErrShort = "Unexpected request"
tmplArgs.ErrLong = "Mismatched state parameter. Possible cross-site-request-forgery?"
return
}
+ if errEnc := r.FormValue("error"); errEnc != "" {
+ tmplArgs.ErrShort = "Failed to get blessings"
+ errBytes, err := base64.URLEncoding.DecodeString(errEnc)
+ if err != nil {
+ tmplArgs.ErrLong = err.Error()
+ } else {
+ tmplArgs.ErrLong = string(errBytes)
+ }
+ return
+ }
result <- r.FormValue("macaroon")
result <- r.FormValue("object_name")
result <- r.FormValue("root_key")
- defer close(result)
blessed, ok := <-blessedChan
if !ok {
tmplArgs.ErrShort = "No blessings received"
diff --git a/services/identity/internal/caveats/browser_caveat_selector.go b/services/identity/internal/caveats/browser_caveat_selector.go
index cbb0400..09746a1 100644
--- a/services/identity/internal/caveats/browser_caveat_selector.go
+++ b/services/identity/internal/caveats/browser_caveat_selector.go
@@ -15,6 +15,8 @@
"v.io/v23/security"
)
+var ErrSeekblessingsCancelled = fmt.Errorf("seekblessings has been cancelled")
+
type browserCaveatSelector struct {
assetsPrefix string
}
@@ -37,11 +39,15 @@
}
func (s *browserCaveatSelector) ParseSelections(r *http.Request) (caveats []CaveatInfo, state string, additionalExtension string, err error) {
+ state = r.FormValue("macaroon")
+ if r.FormValue("cancelled") == "true" {
+ err = ErrSeekblessingsCancelled
+ return
+ }
+ additionalExtension = r.FormValue("blessingExtension")
if caveats, err = s.caveats(r); err != nil {
return
}
- state = r.FormValue("macaroon")
- additionalExtension = r.FormValue("blessingExtension")
return
}
diff --git a/services/identity/internal/oauth/handler.go b/services/identity/internal/oauth/handler.go
index b9d1892..97e962c 100644
--- a/services/identity/internal/oauth/handler.go
+++ b/services/identity/internal/oauth/handler.go
@@ -393,7 +393,8 @@
func (h *handler) sendMacaroon(w http.ResponseWriter, r *http.Request) {
var inputMacaroon addCaveatsMacaroon
caveatInfos, macaroonString, blessingExtension, err := h.args.CaveatSelector.ParseSelections(r)
- if err != nil {
+ cancelled := err == caveats.ErrSeekblessingsCancelled
+ if !cancelled && err != nil {
util.HTTPBadRequest(w, r, fmt.Errorf("failed to parse blessing information: %v", err))
return
}
@@ -401,10 +402,19 @@
util.HTTPBadRequest(w, r, fmt.Errorf("suspected request forgery: %v", err))
return
}
-
+ // Construct the url to send back to the tool.
+ baseURL, err := validLoopbackURL(inputMacaroon.ToolRedirectURL)
+ if err != nil {
+ util.HTTPBadRequest(w, r, fmt.Errorf("invalid ToolRedirectURL: %v", err))
+ return
+ }
+ // Now that we have a valid tool redirect url, we can send the errors to the tool.
+ if cancelled {
+ h.sendErrorToTool(w, r, inputMacaroon.ToolState, baseURL, caveats.ErrSeekblessingsCancelled)
+ }
caveats, err := h.caveats(caveatInfos)
if err != nil {
- util.HTTPBadRequest(w, r, fmt.Errorf("failed to create caveats: %v", err))
+ h.sendErrorToTool(w, r, inputMacaroon.ToolState, baseURL, fmt.Errorf("failed to create caveats: %v", err))
return
}
parts := []string{inputMacaroon.Email}
@@ -412,7 +422,7 @@
parts = append(parts, blessingExtension)
}
if len(caveats) == 0 {
- util.HTTPBadRequest(w, r, fmt.Errorf("server disallows attempts to bless with no caveats"))
+ h.sendErrorToTool(w, r, inputMacaroon.ToolState, baseURL, fmt.Errorf("server disallows attempts to bless with no caveats"))
return
}
m := BlessingMacaroon{
@@ -422,18 +432,12 @@
}
macBytes, err := vom.Encode(m)
if err != nil {
- util.HTTPServerError(w, fmt.Errorf("failed to encode BlessingsMacaroon: %v", err))
- return
- }
- // Construct the url to send back to the tool.
- baseURL, err := validLoopbackURL(inputMacaroon.ToolRedirectURL)
- if err != nil {
- util.HTTPBadRequest(w, r, fmt.Errorf("invalid ToolRedirectURL: %v", err))
+ h.sendErrorToTool(w, r, inputMacaroon.ToolState, baseURL, fmt.Errorf("failed to encode BlessingsMacaroon: %v", err))
return
}
marshalKey, err := h.args.Principal.PublicKey().MarshalBinary()
if err != nil {
- util.HTTPServerError(w, fmt.Errorf("failed to marshal public key: %v", err))
+ h.sendErrorToTool(w, r, inputMacaroon.ToolState, baseURL, fmt.Errorf("failed to marshal public key: %v", err))
return
}
encKey := base64.URLEncoding.EncodeToString(marshalKey)
@@ -446,6 +450,15 @@
http.Redirect(w, r, baseURL.String(), http.StatusFound)
}
+func (h *handler) sendErrorToTool(w http.ResponseWriter, r *http.Request, toolState string, baseURL *url.URL, err error) {
+ errEnc := base64.URLEncoding.EncodeToString([]byte(err.Error()))
+ params := url.Values{}
+ params.Add("error", errEnc)
+ params.Add("state", toolState)
+ baseURL.RawQuery = params.Encode()
+ http.Redirect(w, r, baseURL.String(), http.StatusFound)
+}
+
func (h *handler) caveats(caveatInfos []caveats.CaveatInfo) (cavs []security.Caveat, err error) {
caveatFactories := caveats.NewCaveatFactory()
for _, caveatInfo := range caveatInfos {
diff --git a/services/identity/internal/templates/caveats.go b/services/identity/internal/templates/caveats.go
index eef93ad..79baa0c 100644
--- a/services/identity/internal/templates/caveats.go
+++ b/services/identity/internal/templates/caveats.go
@@ -108,8 +108,9 @@
</div>
<div class="action-buttons">
+ <input type="text" class="hidden" name="cancelled" id="cancelled" value="false">
<button class="button-tertiary" id="cancel" type="button">Cancel</button>
- <button class="button-primary" type="submit">Bless</button>
+ <button class="button-primary" type="submit" id="bless">Bless</button>
</div>
<p class="disclaimer-text">
@@ -194,11 +195,11 @@
$(this).replaceWith('<button type="button" class="button-passive right removeCaveat hidden">Remove</button>');
});
- // Upon clicking submit, caveats that have not been added yet should be removed,
+ // Upon clicking bless, caveats that have not been added yet should be removed,
// before they are sent to the server.
- $('#caveats-form').submit(function(){
+ $('#bless').click(function(){
$('.addCaveat').parents('.caveatRow').remove();
- return true;
+ $("#caveats-form").submit();
});
// Upon clicking the 'Remove Caveat' button, the caveat row should be removed.
@@ -231,7 +232,8 @@
// Activate the cancel button.
$('#cancel').click(function(){
- window.close();
+ $("#cancelled").val("true");
+ $("#caveats-form").submit();
});
});
</script>
