From 7ef12da4fa06953610764fea5eebe3131a765072 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 19 May 2024 08:12:39 +0200 Subject: [PATCH] Fix subject handling for login requests This change implements handling for the case that a login request retrieved from Hydra has a pre-defined subject. The login request is rejected if the requested subject is not part of the presented client certificate. --- README.md | 12 ++++---- internal/handlers/login.go | 58 ++++++++++++++++++++++++++++++++++--- internal/services/i18n.go | 8 +++++ translations/active.de.toml | 8 +++++ translations/active.en.toml | 2 ++ 5 files changed, 78 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 07f1dc2..fdd8f39 100644 --- a/README.md +++ b/README.md @@ -66,26 +66,26 @@ internationalization (i18n) support. The translation workflow needs the `go18n` binary which can be installed via -``` +```shell go install github.com/nicksnyder/go-i18n/v2/goi18n ``` To extract new messages from the code run -``` -goi18n extract . +```shell +goi18n extract -outdir translations . ``` Then use -``` -goi18n merge active.*.toml +```shell +goi18n merge -outdir translations translations/active.*.toml ``` to create TOML files for translation as `translate..toml`. After translating the messages run -``` +```shell goi18n merge active.*.toml translate.*.toml ``` diff --git a/internal/handlers/login.go b/internal/handlers/login.go index 1cdd2ce..473c1c7 100644 --- a/internal/handlers/login.go +++ b/internal/handlers/login.go @@ -110,6 +110,8 @@ func (h *LoginHandler) handleGet( return } + usableEmails := certEmails + defer func() { _ = response.Body.Close() }() h.logger.Debug( @@ -117,7 +119,28 @@ func (h *LoginHandler) handleGet( "response", response.Status, "login_request", oAuth2LoginRequest, ) - h.renderRequestForClientCert(w, r, certEmails, localizer, oAuth2LoginRequest) + if subject, ok := oAuth2LoginRequest.GetSubjectOk(); ok && *subject != "" { + h.logger.Info("oauth2LoginRequest expects subject", "subject", *subject) + + subjectInCert := false + + for _, email := range certEmails { + if *subject == email { + subjectInCert = true + break + } + } + + if !subjectInCert { + h.rejectLoginMissingSubject(w, r, challenge, localizer, *subject) + + return + } + + usableEmails = []string{*subject} + } + + h.renderRequestForClientCert(w, r, usableEmails, localizer, oAuth2LoginRequest) } type FlashMessage struct { @@ -234,7 +257,7 @@ func (h *LoginHandler) rejectLogin( r.Context(), ).LoginChallenge(challenge).RejectOAuth2Request(*rejectRequest).Execute() if err != nil { - h.logger.Error("error getting reject login request", "error", err) + h.logger.Error("error sending reject login request", "error", err) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return @@ -242,8 +265,35 @@ func (h *LoginHandler) rejectLogin( defer func() { _ = response.Body.Close() }() - h.logger.Debug( - "go response for RejectOAuth2LoginRequest", + h.logger.DebugContext( + r.Context(), + "got response for RejectOAuth2LoginRequest", + "response", response.Status, "reject_login_request", rejectLoginRequest, + ) + + w.Header().Set("Location", rejectLoginRequest.GetRedirectTo()) + w.WriteHeader(http.StatusFound) +} + +func (h *LoginHandler) rejectLoginMissingSubject(w http.ResponseWriter, r *http.Request, challenge string, localizer *i18n.Localizer, subject string) { + rejectRequest := client.NewRejectOAuth2RequestWithDefaults() + rejectRequest.SetErrorDescription(h.trans.LookupMessage("LoginDeniedSubjectMissing", map[string]interface{}{"Subject": subject}, localizer)) + rejectRequest.SetErrorHint(h.trans.LookupMessage("HintChooseDifferentClientCertificate", nil, localizer)) + rejectRequest.SetStatusCode(http.StatusForbidden) + + rejectLoginRequest, response, err := h.adminClient.RejectOAuth2LoginRequest(r.Context()).LoginChallenge(challenge).RejectOAuth2Request(*rejectRequest).Execute() + if err != nil { + h.logger.Error("error sending reject login request", "error", err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + + return + } + + defer func() { _ = response.Body.Close() }() + + h.logger.DebugContext( + r.Context(), + "got response for RejectOAuth2LoginRequest", "response", response.Status, "reject_login_request", rejectLoginRequest, ) diff --git a/internal/services/i18n.go b/internal/services/i18n.go index 51f6238..d28c6de 100644 --- a/internal/services/i18n.go +++ b/internal/services/i18n.go @@ -312,6 +312,10 @@ func (s *I18NService) AddMessages() error { ID: "LoginDeniedByUser", Other: "Login has been denied by the user.", } + messages["LoginDeniedSubjectMissing"] = &i18n.Message{ + ID: "LoginDeniedSubjectMissing", + Other: "Login has been denied because the requested subject {{ .Subject }} could not be authenticated.", + } messages["LogoutSuccessfulTitle"] = &i18n.Message{ ID: "LogoutSuccessfulTitle", Other: "Logout successful", @@ -332,6 +336,10 @@ func (s *I18NService) AddMessages() error { ID: "HintChooseAnIdentityForAuthentication", Other: "Choose an identity for authentication.", } + messages["HintChooseDifferentClientCertificate"] = &i18n.Message{ + ID: "HintChooseDifferentClientCertificate", + Other: "Choose a different client certificate for authentication.", + } messages["NoConsentGiven"] = &i18n.Message{ ID: "NoConsentGiven", Other: "You have not given consent to use your data to any application yet.", diff --git a/translations/active.de.toml b/translations/active.de.toml index b758293..62215a6 100644 --- a/translations/active.de.toml +++ b/translations/active.de.toml @@ -93,6 +93,10 @@ other = "Unbekannter Fehler" hash = "sha1-7ee5b067009bbedc061274ee50a3027b50a06163" other = "Wähle eine Identität für die Anmeldung." +[HintChooseDifferentClientCertificate] +hash = "sha1-22002121759ae58afccfccb88383dbe54f94f0a9" +other = "Wähle ein anderes Client-Zertifikat für die Anmeldung." + [IndexTitle] hash = "sha1-c763022b69a8ad58ab42d8ea708192abd85fd8f6" other = "Willkommen bei deinem Identitätsprovider" @@ -139,6 +143,10 @@ other = "Unbekannt" hash = "sha1-bbad650536bfb091ad55d576262bbe4358277c73" other = "Die Anmeldung wurde durch den Nutzer abgelehnt." +[LoginDeniedSubjectMissing] +hash = "sha1-c787e750515612fd695d6a6beeb3a07ec381f3e7" +other = "Die Anmeldung wurde abgelehnt, weil die angeforderte Identität {{ .Subject }} nicht bestätigt werden konnte." + [LoginTitle] hash = "sha1-9a24c8b64e047edc13f3c41ef7785bb2044a6d69" other = "Anmelden mit einem Client-Zertifikat" diff --git a/translations/active.en.toml b/translations/active.en.toml index 034ca40..62e1d38 100644 --- a/translations/active.en.toml +++ b/translations/active.en.toml @@ -8,6 +8,7 @@ ConfirmRevokeTitle = "Revoke consent" ErrorTitle = "An error has occurred" ErrorUnknown = "Unknown error" HintChooseAnIdentityForAuthentication = "Choose an identity for authentication." +HintChooseDifferentClientCertificate = "Choose a different client certificate for authentication." IndexTitle = "Welcome to your identity provider" IndexWelcomeMessage = "Go to [manage consent]({{ .ManageConsentHRef }}) to show or revoke consent you have given to client applications." IntroConsentMoreInformation = "You can find more information about **{{ .client }}** at [its description page]({{ .clientLink }})." @@ -17,6 +18,7 @@ LabelNever = "Never" LabelSubmit = "Submit" LabelUnknown = "Unknown" LoginDeniedByUser = "Login has been denied by the user." +LoginDeniedSubjectMissing = "Login has been denied because the requested subject {{ .Subject }} could not be authenticated." LoginTitle = "Authenticate with a client certificate" LogoutSuccessfulText = "You have been logged out successfully." LogoutSuccessfulTitle = "Logout successful"