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.

Sign up for GitHub

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

Jump to bottom

remove unnecessary warning log message #2010

Merged
merged 1 commit into from
Jul 12, 2024
Merged

remove unnecessary warning log message #2010

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 2 additions & 22 deletions internal/federationdomain/endpoints/auth/auth_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,13 @@ func (h *authorizeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

h.authorize(w, r, requestedBrowserlessFlow, idpNameQueryParamValue, idp)
h.authorize(w, r, requestedBrowserlessFlow, idp)
}

func (h *authorizeHandler) authorize(
w http.ResponseWriter,
r *http.Request,
requestedBrowserlessFlow bool,
idpNameQueryParamValue string,
idp resolvedprovider.FederationDomainResolvedIdentityProvider,
) {
// Browser flows do not need session storage at this step. For browser flows, the request parameters
Expand All @@ -165,8 +164,6 @@ func (h *authorizeHandler) authorize(
return
}

maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester)

// Automatically grant certain scopes, but only if they were requested.
// Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
// There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope
Expand Down Expand Up @@ -304,8 +301,7 @@ func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {
return csrfFromCookie
}

// chooseUpstreamIDP selects either an OIDC, an LDAP, or an AD IDP, or returns an error.
// Note that AD and LDAP IDPs both return the same interface type, but different ProviderTypes values.
// chooseUpstreamIDP selects an upstream IDP, or returns an error.
func chooseUpstreamIDP(idpDisplayName string, idpLister federationdomainproviders.FederationDomainIdentityProvidersFinderI) (
resolvedprovider.FederationDomainResolvedIdentityProvider,
error,
Expand All @@ -320,22 +316,6 @@ func chooseUpstreamIDP(idpDisplayName string, idpLister federationdomainprovider
return idpLister.FindUpstreamIDPByDisplayName(idpDisplayName)
}

func maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue string, authorizeRequester fosite.AuthorizeRequester) {
if len(idpNameQueryParamValue) != 0 {
return
}
plog.Warning("Client attempted to perform an authorization flow (user login) without specifying the "+
"query param to choose an identity provider. "+
"This will not work when identity providers are configured explicitly on a FederationDomain. "+
"Additionally, this behavior is deprecated and support for any authorization requests missing this query param "+
"may be removed in a future release. "+
"Please ask the author of this client to update the authorization request URL to include this query parameter. "+
"The value of the parameter should be equal to the displayName of the identity provider as declared in the FederationDomain.",
"missingParameterName", oidcapi.AuthorizeUpstreamIDPNameParamName,
"clientID", authorizeRequester.GetClient().GetID(),
)
}

// generateUpstreamAuthorizeRequestState performs the shared validations and setup between browser based
// auth requests regardless of IDP type.
// It generates the state param, sets the CSRF cookie, and validates the prompt param.
Expand Down
Loading