Fixed renewal and deletion of supergroup entries

builtin/credential/appgroup/path_login.go

@@ -30,20 +30,6 @@ func pathLogin(b *backend) *framework.Path {
 	}
 }
 
-func (b *backend) pathLoginRenew(req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
-	selectorType := req.Auth.InternalData["selector_type"].(string)
-	selectorValue := req.Auth.InternalData["selector_value"].(string)
-	if selectorType == "" || selectorValue == "" {
-		return nil, fmt.Errorf("failed to fetch selector type and/or selector value during renewal")
-	}
-	resp, err := b.validateSelector(req.Storage, selectorType, selectorValue)
-	if err != nil {
-		return nil, fmt.Errorf("failed to validate selector during renewal:%s", err)
-	}
-
-	return framework.LeaseExtend(resp.TokenTTL, resp.TokenMaxTTL, b.System())(req, data)
-}
-
 func (b *backend) pathLoginUpdate(req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	secretID := strings.TrimSpace(data.Get("secret_id").(string))
 	if secretID == "" {
@@ -71,8 +57,10 @@ func (b *backend) pathLoginUpdate(req *logical.Request, data *framework.FieldDat
 
 	resp := &logical.Response{
 		Auth: &logical.Auth{
-			InternalData: map[string]interface{}{},
-			Policies:     validateResp.Policies,
+			InternalData: map[string]interface{}{
+				"selector_id": validateResp.SelectorID,
+			},
+			Policies: validateResp.Policies,
 			LeaseOptions: logical.LeaseOptions{
 				TTL:       validateResp.TokenTTL,
 				Renewable: true,
@@ -82,6 +70,20 @@ func (b *backend) pathLoginUpdate(req *logical.Request, data *framework.FieldDat
 	return resp, nil
 }
 
+func (b *backend) pathLoginRenew(req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+	selectorID := req.Auth.InternalData["selector_id"].(string)
+	if selectorID == "" {
+		return nil, fmt.Errorf("failed to fetch selector_id during renewal")
+	}
+
+	resp, err := b.validateSelectorID(req.Storage, selectorID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to validate selector during renewal:%s", err)
+	}
+
+	return framework.LeaseExtend(resp.TokenTTL, resp.TokenMaxTTL, b.System())(req, data)
+}
+
 const pathLoginHelpSys = "Issue a token for a given pair of 'selector' and 'secret_id'."
 
 const pathLoginHelpDesc = `The supplied SecretID could've been generated/assigned against an

builtin/credential/appgroup/path_login_test.go

@@ -1,7 +1,6 @@
 package appgroup
 
 import (
-	"log"
 	"testing"
 
 	"github.com/hashicorp/vault/helper/policies"
@@ -23,8 +22,6 @@ func TestBackend_supergroup_login(t *testing.T) {
 	createGroup(t, b, storage, "group2", "app5,app6", "o,p")
 	createGroup(t, b, storage, "group3", "app3,app4,app5,app6", "q,r")
 
-	log.Printf("creating super group now\n")
-
 	superGroupSecretIDData := map[string]interface{}{
 		"groups":              "group1,group2,group3",
 		"apps":                "app1,app2",
@@ -51,7 +48,6 @@ func TestBackend_supergroup_login(t *testing.T) {
 		"selector_id": resp.Data["selector_id"],
 		"secret_id":   resp.Data["secret_id"],
 	}
-	log.Printf("loginData: %#v\n", loginData)
 	loginReq := &logical.Request{
 		Operation: logical.UpdateOperation,
 		Path:      "login",

builtin/credential/appgroup/path_supergroup.go

@@ -2,6 +2,7 @@ package appgroup
 
 import (
 	"fmt"
+	"log"
 	"strings"
 	"time"
 
@@ -161,6 +162,7 @@ addition to those, a set of policies can be assigned using this.
 func (b *backend) setSuperGroupEntry(s logical.Storage, superGroupName string, superGroup *superGroupStorageEntry) error {
 	b.superGroupLock.Lock()
 	defer b.superGroupLock.Unlock()
+	log.Printf("reading supergroup: %s\n", superGroupName)
 	entry, err := logical.StorageEntryJSON("supergroup/"+strings.ToLower(superGroupName), superGroup)
 	if err != nil {
 		return err
@@ -195,6 +197,7 @@ func (b *backend) superGroupEntry(s logical.Storage, superGroupName string) (*su
 	b.superGroupLock.RLock()
 	defer b.superGroupLock.RUnlock()
 
+	log.Printf("reading supergroup: %s\n", superGroupName)
 	if entry, err := s.Get("supergroup/" + strings.ToLower(superGroupName)); err != nil {
 		return nil, err
 	} else if entry == nil {

builtin/credential/appgroup/secret_id.go

@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
+	"log"
 	"sync"
 	"time"
 
@@ -45,12 +46,11 @@ type secretIDStorageEntry struct {
 // required to be stored as metadata in the response, and/or the information required to
 // create the client token.
 type validationResponse struct {
-	SelectorID    string        `json:"selector_id" structs:"selector_id" mapstructure:"selector_id"`
-	HMACKey       string        `json:"hmac_key" structs:"hmac_key" mapstructure:"hmac_key"`
-	SelectorValue string        `json:"selector_value" structs:"selector_value" mapstructure:"selector_value"`
-	TokenTTL      time.Duration `json:"token_ttl" structs:"token_ttl" mapstructure:"token_ttl"`
-	TokenMaxTTL   time.Duration `json:"token_max_ttl" structs:"token_max_ttl" mapstructure:"token_max_ttl"`
-	Policies      []string      `json:"policies" structs:"policies" mapstructure:"policies"`
+	SelectorID  string        `json:"selector_id" structs:"selector_id" mapstructure:"selector_id"`
+	HMACKey     string        `json:"hmac_key" structs:"hmac_key" mapstructure:"hmac_key"`
+	TokenTTL    time.Duration `json:"token_ttl" structs:"token_ttl" mapstructure:"token_ttl"`
+	TokenMaxTTL time.Duration `json:"token_max_ttl" structs:"token_max_ttl" mapstructure:"token_max_ttl"`
+	Policies    []string      `json:"policies" structs:"policies" mapstructure:"policies"`
 }
 
 // selectorStorageEntry represents the reverse mapping of the selector to
@@ -123,15 +123,7 @@ func (b *backend) validateCredentials(s logical.Storage, selectorID, secretID st
 		return nil, fmt.Errorf("missing secret_id")
 	}
 
-	selIDEntry, err := b.selectorIDEntry(s, selectorID)
-	if err != nil {
-		return nil, err
-	}
-	if selIDEntry == nil {
-		return nil, fmt.Errorf("invalid selector_id")
-	}
-
-	validationResp, err := b.validateSelector(s, selIDEntry.Type, selIDEntry.Name)
+	validationResp, err := b.validateSelectorID(s, selectorID)
 	if err != nil {
 		return nil, err
 	}
@@ -150,29 +142,38 @@ func (b *backend) validateCredentials(s logical.Storage, selectorID, secretID st
 }
 
 // Check if there exists an entry in the name of selectorValue for the selectorType supplied.
-func (b *backend) validateSelector(s logical.Storage, selectorType, selectorValue string) (*validationResponse, error) {
+func (b *backend) validateSelectorID(s logical.Storage, selectorID string) (*validationResponse, error) {
+	selector, err := b.selectorIDEntry(s, selectorID)
+	if err != nil {
+		return nil, err
+	}
+	if selector == nil {
+		return nil, fmt.Errorf("failed to find selector for selector_id:%s\n", selectorID)
+	}
+	log.Printf("selector:%#v\n", selector)
+
 	resp := &validationResponse{}
-	switch selectorType {
+	switch selector.Type {
 	case selectorTypeApp:
-		app, err := b.appEntry(s, selectorValue)
+		app, err := b.appEntry(s, selector.Name)
 		if err != nil {
 			return nil, err
 		}
 		if app == nil {
-			return nil, fmt.Errorf("app referred by the secret ID does not exist")
+			return nil, fmt.Errorf("app %s referred by the secret ID does not exist", selector.Name)
 		}
 		resp.Policies = app.Policies
 		resp.TokenTTL = app.TokenTTL
 		resp.TokenMaxTTL = app.TokenMaxTTL
 		resp.SelectorID = app.SelectorID
 		resp.HMACKey = app.HMACKey
 	case selectorTypeGroup:
-		group, err := b.groupEntry(s, selectorValue)
+		group, err := b.groupEntry(s, selector.Name)
 		if err != nil {
 			return nil, err
 		}
 		if group == nil {
-			return nil, fmt.Errorf("group referred by the secret ID does not exist")
+			return nil, fmt.Errorf("group %s referred by the secret ID does not exist", selector.Name)
 		}
 		groupPolicies, err := b.fetchPolicies(s, group.Apps)
 		if err != nil {
@@ -189,7 +190,7 @@ func (b *backend) validateSelector(s logical.Storage, selectorType, selectorValu
 		resp.SelectorID = group.SelectorID
 		resp.HMACKey = group.HMACKey
 	case selectorTypeSuperGroup:
-		superGroup, err := b.superGroupEntry(s, selectorValue)
+		superGroup, err := b.superGroupEntry(s, selector.Name)
 		if err != nil {
 			return nil, err
 		}
@@ -232,7 +233,6 @@ func (b *backend) validateSelector(s logical.Storage, selectorType, selectorValu
 	}
 
 	// Cap the token_ttl and token_max_ttl values.
-	var err error
 	resp.TokenTTL, resp.TokenMaxTTL, err = b.SanitizeTTL(resp.TokenTTL, resp.TokenMaxTTL)
 	if err != nil {
 		return nil, err
@@ -310,13 +310,19 @@ func (b *backend) secretIDEntryValid(s logical.Storage, selectorID, secretID, hm
 		// and it is not cleaned up in any other way. When the SecretID belonging
 		// to the superGroup storage entry is getting invalidated, the entry should
 		// be deleted as well.
-		/*
-			if selectorType == selectorTypeSuperGroup {
-				if err := b.deleteSuperGroupEntry(s, selectorValue); err != nil {
-					return false, err
-				}
+		selector, err := b.selectorIDEntry(s, selectorID)
+		if err != nil {
+			return false, err
+		}
+		if selector == nil {
+			return false, fmt.Errorf("failed to find selector for selector_id:%s\n", selectorID)
+		}
+
+		if selector.Type == selectorTypeSuperGroup {
+			if err := b.deleteSuperGroupEntry(s, selector.Name); err != nil {
+				return false, err
 			}
-		*/
+		}
 	} else {
 		// If the use count is greater than one, decrement it and update the last updated time.
 		result.SecretIDNumUses -= 1