Background: For dynamic clients, the groups scope is not always allowed
and/or requested by the client, so it will not always be granted by the
Supervisor for an authorization request.
Previously, when the groups scope was not granted, we would skip
searching for upstream groups in some scenarios.
This commit changes the behavior of authorization flows so that even
when the groups scope is not granted we still search for the upstream
group memberships as configured, and we pass the upstream group
memberships into any configured identity transformations. The identity
transformations could potentially reject the user's authentication based
on their upstream group membership.
When the groups scope is not granted, we don't include the groups in
the final Supervisor-issued ID token. This behavior is not changed.
Each endpoint handler is now responsible for applying the identity
transformations and creating most of the session data, rather than each
implementation of the upstream IDP interface. This shares code better,
and reduces the responsibilities of the implementations of the IDP
interface by letting them focus more on the upstream stuff.
Also refactor the parameters and return types of the IDP interfaces to
make them more clear, and because they can be more focused on upstream
identities (pre-identity transformation). This clarifies the
responsibilities of the implementations of the IDP interface.
Create an interface to abstract the upstream IDP from the
authorize, IDP discovery, callback, choose IDP, and login
endpoints. This commit does not refactor the token endpoint,
which will be refactored in a similar way in the next commit.
- continued refactoring the auth handler to share more code between
the two supported browserless flows: OIDC and LDAP/AD
- the upstreamldap package should not know about the concept of
OIDC granted scopes, so refactored it to be a skipGroups bool
- Simplify the error handling in the authorize endpoint by making the
private helper functions return fosite-style errors, and having
one place that writes those errors to the response.
- Some types of errors were previously returned as regular http-style
errors. Those have all been converted to be returned as oauth-style
errors (which can be redirects to the client), except for http method
not found errors. This is a change in behavior from the client's point
of view, but only when those unexpected errors happen. These types of
errors are more consistent with RFC6749 section 4.1.2.1.
- Avoids using the httperr package for error handling.
- Create a struct for the handler as a first step toward making smaller
functions with fewer parameters.
In the RFC8693 token exchange, the CLI sends your access token and
receives in exchange a new cluster-scoped ID token.
Fix a bug in the CLI. Whenever the "pinniped login oidc" command was
planning to perform the RFC8693 token exchange, it failed to check if
the cached access token was still valid before performing the exchange,
which sends the access token. It instead checked if the cached ID token
was still valid, but that it not relevant in this situation because the
ID token is not going to be used for anything (instead the new ID token
returned by the RFC8693 token exchange will be used for auth).
This bug doesn't actually matter today, because the Supervisor-issued
access and ID tokens always both have the same 2-minute lifetimes.
However, future enhancements may cause them to have different lifetimes
in certain circumstances. Fixing this CLI bug now to prepare for those
potential future enhancements.
Goboring only allows TLS 1.2.
The next goboring will allow both TLS 1.2 and TLS 1.3. We got a preview
of this when the Go team upgraded goboring in Go 1.21.6, but then
downgraded it again in the next Go releases.
When the Go team eventually upgrades goboring again, then we can
revert this commit to bring back TLS 1.3 support in FIPS mode.
- Current goboring only allows TLS 1.2.
- The next goboring will allow TLS 1.2 and TLS 1.3. We got a preview
of this when the Go team upgraded goboring in Go 1.21.6, but then
downgraded it again in the next Go releases.
The release of Go 1.21.6 includes the new boring crypto when compiling
with FIPS enabled. See https://go.dev/doc/devel/release#go1.21.0 and
https://github.com/golang/go/issues/64717.
This new version of boring crypto allows the use of TLS v1.3 for the
first time, so we changed the Pinniped code to use TLS v1.3 where
appropriate when compiled with the FIPS compiler. It also changed the
allowed TLS v1.2 ciphers, so we updated those as well.
After this commit, the project must be compiled by at least Go v1.21.6
when compiling in fips mode. The hack/Dockerfile_fips was already
updated to use that version of Go in a previous commit.
Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
A small part of the session storage changed type in the latest version
of fosite compared to the old version of fosite that we were using.
Just to be safe, update our session storage version to invalidate
any pre-existing sessions upon upgrade of Pinniped.
Made the switch wherever possible, but since fosite still uses the old
gopkg.in/square/go-jose.v2 there was one test where we still need to use
it as a direct dependency.
The unused-parameter linter became stricter, so we adjust it to
allow unused params that start with underscore. It can be nice to keep
unused param names when implementing an interface sometimes, to help
readers understand why it is unused in that particular implementation.
Before this fix, the deadlock would prevent the leader pod from giving
up its lease, which would make it take several minutes for new pods to
be allowed to elect a new leader. During that time, no Pinniped
controllers could write to the Kube API, so important resources were not
being updated during that window. It would also make pod shutdown take
about 1 minute.
After this fix, the leader gives up its lease immediately, and pod
shutdown takes about 1 second. This improves restart/upgrade time and
also fixes the problem where there was no leader for several minutes
after a restart/upgrade.
The deadlock was between the post-start hook and the pre-shutdown hook.
The pre-shutdown hook blocked until a certain background goroutine in
the post-start hook finished, but that goroutine could not finish until
the pre-shutdown hook finished. Thus, they were both blocked, waiting
for each other infinitely. Eventually the process would be externally
killed.
This deadlock was most likely introduced by some change in Kube's
generic api server package related to how the many complex channels used
during server shutdown interact with each other, and was not noticed
when we upgraded to the version which introduced the change.
To make the subject of the downstream ID token more unique when
there are multiple IDPs. It is possible to define two IDPs in a
FederationDomain using the same identity provider CR, in which
case the only thing that would make the subject claim different
is adding the IDP display name into the values of the subject claim.
- Remove that validation from the controller since the CRD already
validates it during creates and updates.
- Also finish the supervisor_federationdomain_status_test.go by adding
more tests for both controller validations and CRD validations
