Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend payload #2876

Open
2 tasks done
loafoe opened this issue Mar 27, 2023 · 3 comments
Open
2 tasks done

Extend payload #2876

loafoe opened this issue Mar 27, 2023 · 3 comments

Comments

@loafoe
Copy link

loafoe commented Mar 27, 2023

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

Dex has very rigid claims support. Adding additional claims via a connector requires forking the code and making changes to various internal structures. There are a number of proposals where this discussed, namely #1635 . The scope of these is fairly large and we are exploring a possible solution that limits itself to just mutating of the claims.

Proposed Solution

We propose adding a new PayloadExtender interface which connectors can choose to implement:

// PayloadExtender allows connectors to enhance the payload before signing
type PayloadExtender interface {
	ExtendPayload(scopes []string, payload []byte, connectorData []byte) ([]byte, error)
}

By implementing this interface connectors get a chance to mutate the id_token payload just before it is signed. The scopes may be used to perform conditional mutation. The payload is passed as a byte array as well as the connectorData associated with the authorization request. This allows the connector to pass context via connectorData. The resulting mutated structure is returned including an error condition.

A working example of this interface implementation could look like this (actual working code):

func (c *hsdpConnector) ExtendPayload(scopes []string, payload []byte, cdata []byte) ([]byte, error) {
	var cd connectorData
	var originalClaims map[string]interface{}

	c.logger.Info("ExtendPayload called")

	if err := json.Unmarshal(cdata, &cd); err != nil {
		return payload, err
	}
	if err := json.Unmarshal(payload, &originalClaims); err != nil {
		return payload, err
	}
	
	// Experimental teams
	var teams []string
	teams = append(teams, cd.Introspect.Organizations.ManagingOrganization)
	originalClaims["teams"] = teams

	extendedPayload, err := json.Marshal(originalClaims)
	if err != nil {
		return payload, err
	}
	return extendedPayload, nil
}

This results in a teams claim being available in the id_token. A working implementation can be found in this fork.

Alternatives Considered

Alternatives are to maintain a fork with these changes.

Additional Information

There are only minimal (non-breaking) changes required to the core dex to support this, essentially changes to propagate the connectorData to the token generation logic. We have successfully tested this approach. If this is interesting I'm happy to write a Dex Feature Proposal which further details.

@loafoe loafoe closed this as not planned Won't fix, can't repro, duplicate, stale Mar 27, 2023
@loafoe loafoe reopened this Mar 27, 2023
@sagikazarmark
Copy link
Member

Thank you for opening this @loafoe!

Indeed a solution with a smaller scope may be easier to incorporate into Dex, but as part of the external connector effort (#1907) the connector interface maybe redesigned and this feature maybe replaced with something else (eg. middleware).

In any case, this proposal is worth exploring.

Would you mind opening an enhancement proposal?

I also wonder if it would make sense at this point to introduce an internal representation for the payload. That could serve as a basis for future improvements, like custom claims.

@loafoe
Copy link
Author

loafoe commented May 17, 2023

Hi @sagikazarmark sorry for the delay. Just submitted a PR #2954 for the initial draft of the proposal. Feedback appreciated!

@kromanow94
Copy link

It'd be great to have that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants