From 2ddc7cf47a54c34f9720d3fa78f2b387b5a9e17b Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Mon, 29 Oct 2018 16:47:44 +0000
Subject: [PATCH] Drop the groups param from the exchange API

There seems to be no good reason to change group membership when
exchanging tokens, let's try just passing the old one over.
---
 README.md         |  8 ++++++--
 server/http.go    |  3 +--
 server/service.go | 24 +++++++++++++-----------
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 9bd4afb..341f91a 100644
--- a/README.md
+++ b/README.md
@@ -115,13 +115,17 @@ parameters:
 * `cur_nonce`: nonce for *cur_tkt*
 * `new_svc`: destination SSO service
 * `new_nonce`: nonce for the new SSO ticket
-* `new_groups` (optional): a comma-separated list of groups that the
-  destination service might check membership for
 
 Note that annoyingly *cur_svc* and *cur_nonce* are redundant, as they
 are already contained within *cur_tkt*, but the SSO ticket API won't
 allow us to decode the ticket without verifying it at the same time.
 
+The new ticket will not be valid any longer than the original one, or
+the configured TTL for the new service, whichever comes first.
+
+Group membership in the original ticket is passed along unchanged to
+the new ticket.
+
 
 # Implementation notes
 
diff --git a/server/http.go b/server/http.go
index 93cc0b6..f906689 100644
--- a/server/http.go
+++ b/server/http.go
@@ -287,9 +287,8 @@ func (h *Server) handleExchange(w http.ResponseWriter, req *http.Request) {
 	curNonce := req.FormValue("cur_nonce")
 	newService := req.FormValue("new_svc")
 	newNonce := req.FormValue("new_nonce")
-	reqGroups := strings.Split(req.FormValue("new_groups"), ",")
 
-	token, err := h.loginService.Exchange(curToken, curService, curNonce, newService, newNonce, reqGroups)
+	token, err := h.loginService.Exchange(curToken, curService, curNonce, newService, newNonce)
 	switch {
 	case err == ErrUnauthorized:
 		log.Printf("unauthorized exchange request (%s -> %s)", curService, newService)
diff --git a/server/service.go b/server/service.go
index d26e04d..f03d165 100644
--- a/server/service.go
+++ b/server/service.go
@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"regexp"
 	"strings"
+	"time"
 
 	"git.autistici.org/id/go-sso"
 )
@@ -86,7 +87,7 @@ func (s *LoginService) Authorize(username, service, destination, nonce string, g
 }
 
 // Exchange a token for a new one scoped to a different service.
-func (s *LoginService) Exchange(curToken, curService, curNonce, newService, nonce string, requiredGroups []string) (string, error) {
+func (s *LoginService) Exchange(curToken, curService, curNonce, newService, nonce string) (string, error) {
 	// Check that the current token is valid for the current service.
 	curTkt, err := s.validator.Validate(curToken, curNonce, curService, nil)
 	if err != nil {
@@ -97,18 +98,19 @@ func (s *LoginService) Exchange(curToken, curService, curNonce, newService, nonc
 	if err := s.validateServiceExchange(curService, newService); err != nil {
 		return "", err
 	}
-	// Match the original service groups against the new service
-	// required groups.
-	var groups []string
-	if requiredGroups != nil {
-		groups = intersectGroups(curTkt.Groups, requiredGroups)
-		if groups == nil {
-			return "", ErrUnauthorized
-		}
+
+	// The new ticket will not be valid any longer than the
+	// original one, or for a maximum of whatever service-specific
+	// TTL we have configured, whichever comes first.
+	ttlLeft := time.Until(curTkt.Expires)
+	if svcTTL := s.config.getServiceTTL(newService); svcTTL < ttlLeft {
+		ttlLeft = svcTTL
 	}
 
-	// Generate a new signed ticket for the new service.
-	tkt := sso.NewTicket(curTkt.User, newService, s.config.Domain, nonce, groups, s.config.getServiceTTL(newService))
+	// Generate a new signed ticket for the new service. Groups
+	// in the original ticket are passed identically to the newly
+	// generated ticket.
+	tkt := sso.NewTicket(curTkt.User, newService, s.config.Domain, nonce, curTkt.Groups, ttlLeft)
 	return s.signer.Sign(tkt)
 }
 
-- 
GitLab