From 56ff01600fb3046480730ba537c4373d1dea6469 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Mon, 7 Aug 2023 17:38:57 +0200 Subject: [PATCH] Improve consent handling - hide client logo if there is no logo URI - hide client information link if there is no client URI - use buttons instead of a checkbox for consent - use Markdown for messages --- changelog.md | 6 +++ internal/handlers/consent.go | 60 ++++++++++++++------------ internal/services/i18n.go | 22 +++++++--- translations/active.de.toml | 22 +++++++--- translations/active.en.toml | 14 ++++-- ui/templates/client_certificate.gohtml | 2 +- ui/templates/consent.gohtml | 20 ++++----- 7 files changed, 92 insertions(+), 54 deletions(-) diff --git a/changelog.md b/changelog.md index c69e211..6e0b3fa 100644 --- a/changelog.md +++ b/changelog.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Changed +- HTML for client logo image is only rendered if a client application has a logo URL +- link to client information is only rendered if a client application has a client URL +- use Consent and Deny buttons instead of a checkbox when asking for consent + ## [0.3.0] - 2023-08-07 ### Changed - use a session to transport data from the login to the consent screens diff --git a/internal/handlers/consent.go b/internal/handlers/consent.go index d2e8cce..8eb347b 100644 --- a/internal/handlers/consent.go +++ b/internal/handlers/consent.go @@ -49,7 +49,7 @@ type ConsentHandler struct { type ConsentInformation struct { GrantedScopes []string `form:"scope"` SelectedClaims []string `form:"claims"` - ConsentChecked bool `form:"consent"` + ConsentAction string `form:"consent"` } type UserInfo struct { @@ -188,7 +188,7 @@ func (h *ConsentHandler) handlePost( return } - if consentInfo.ConsentChecked { + if consentInfo.ConsentAction == "consent" { consentRequest, err := h.rememberNewConsent(consentData, consentInfo, requestedClaims, session) if err != nil { h.logger.WithError(err).Error("could not accept consent") @@ -357,36 +357,42 @@ func (h *ConsentHandler) renderConsentForm( claims *models.OIDCClaimsRequest, localizer *i18n.Localizer, ) { - trans := func(id string, values ...map[string]interface{}) string { - if len(values) > 0 { - return h.trans.LookupMessage(id, values[0], localizer) - } - - return h.trans.LookupMessage(id, nil, localizer) + trans := h.trans.LookupMessage + transMarkdown := func(id string, params map[string]interface{}, localizer *i18n.Localizer) template.HTML { + return template.HTML( //nolint:gosec + h.trans.LookupMarkdownMessage(id, params, localizer), + ) } // render consent form oAuth2Client := consentRequest.Client + clientLogoURI := oAuth2Client.GetLogoUri() + clientName := template.HTMLEscaper(oAuth2Client.GetClientName()) + clientURI := oAuth2Client.GetClientUri() + h.templates.render(h.logger, w, Consent, map[string]interface{}{ - "Title": trans("TitleRequestConsent"), - csrf.TemplateTag: csrf.TemplateField(r), - "errors": map[string]string{}, - "client": oAuth2Client, - "requestedScope": h.mapRequestedScope(consentRequest.RequestedScope, localizer), - "requestedClaims": h.mapRequestedClaims(claims, localizer), - "LabelSubmit": trans("LabelSubmit"), - "LabelConsent": trans("LabelConsent"), - "IntroMoreInformation": template.HTML( //nolint:gosec - trans("IntroConsentMoreInformation", map[string]interface{}{ - "client": oAuth2Client.GetClientName(), - "clientLink": oAuth2Client.GetClientUri(), - })), - "ClaimsInformation": template.HTML( //nolint:gosec - trans("ClaimsInformation", nil)), - "IntroConsentRequested": template.HTML( //nolint:gosec - trans("IntroConsentRequested", map[string]interface{}{ - "client": oAuth2Client.GetClientName(), - })), + "Title": trans("TitleRequestConsent", nil, localizer), + csrf.TemplateTag: csrf.TemplateField(r), + "errors": map[string]string{}, + "LogoURI": clientLogoURI, + "ClientName": clientName, + "requestedScope": h.mapRequestedScope(consentRequest.RequestedScope, localizer), + "requestedClaims": h.mapRequestedClaims(claims, localizer), + "ButtonTitleConsent": trans("ButtonTitleConsent", nil, localizer), + "ButtonTitleDeny": trans("ButtonTitleDeny", nil, localizer), + "HasMoreInformation": clientURI != "", + "IntroMoreInformation": transMarkdown( + "IntroConsentMoreInformation", map[string]interface{}{ + "client": clientName, + "clientLink": clientURI, + }, localizer), + "LabelConsent": transMarkdown("LabelConsent", nil, localizer), + "ClaimsInformation": transMarkdown( + "ClaimsInformation", nil, localizer), + "IntroConsentRequested": transMarkdown( + "IntroConsentRequested", map[string]interface{}{ + "client": clientName, + }, localizer), }) } diff --git a/internal/services/i18n.go b/internal/services/i18n.go index 705683b..0993c01 100644 --- a/internal/services/i18n.go +++ b/internal/services/i18n.go @@ -181,6 +181,16 @@ func (s *I18NService) AddMessages() error { Description: "Title for a button to cancel an action", Other: "Cancel", } + messages["ButtonTitleConsent"] = &i18n.Message{ + ID: "ButtonTitleConsent", + Description: "Title for a button to give consent", + Other: "Consent", + } + messages["ButtonTitleDeny"] = &i18n.Message{ + ID: "ButtonTitleDeny", + Description: "Title for a button to deny consent", + Other: "Deny", + } messages["ButtonTitleRevoke"] = &i18n.Message{ ID: "ButtonTitleRevoke", Description: "Title for a button to revoke consent", @@ -246,14 +256,12 @@ func (s *I18NService) AddMessages() error { Other: "Unknown", } messages["IntroConsentRequested"] = &i18n.Message{ - ID: "IntroConsentRequested", - Other: "The {{ .client }} application requested your consent for the following set of " + - "permissions:", + ID: "IntroConsentRequested", + Other: "The **{{ .client }}** application requested your consent for the following set of permissions:", } messages["IntroConsentMoreInformation"] = &i18n.Message{ - ID: "IntroConsentMoreInformation", - Other: "You can find more information about {{ .client }} at " + - "its description page.", + ID: "IntroConsentMoreInformation", + Other: "You can find more information about **{{ .client }}** at [its description page]({{ .clientLink }}).", } messages["ClaimsInformation"] = &i18n.Message{ ID: "ClaimsInformation", @@ -265,7 +273,7 @@ func (s *I18NService) AddMessages() error { } messages["CertLoginIntroText"] = &i18n.Message{ ID: "CertLoginIntroText", - Other: "The application {{ .ClientName }} requests a login.", + Other: "The application **{{ .ClientName }}** requests a login.", } messages["EmailChoiceText"] = &i18n.Message{ ID: "EmailChoiceText", diff --git a/translations/active.de.toml b/translations/active.de.toml index 6b112c4..b758293 100644 --- a/translations/active.de.toml +++ b/translations/active.de.toml @@ -16,14 +16,24 @@ description = "Title for a button to confirm consent revocation" hash = "sha1-bb25839982a1fe044fc2ac39552903c65402ad48" other = "Ja, Widerrufen!" +[ButtonTitleConsent] +description = "Title for a button to give consent" +hash = "sha1-7f5c5d105e7a9600c7a8d3d092c7e812cf344e95" +other = "Zustimmen" + +[ButtonTitleDeny] +description = "Title for a button to deny consent" +hash = "sha1-d5245e4b19ed6f0af002aee7ab488b8f822d53c7" +other = "Ablehnen" + [ButtonTitleRevoke] description = "Title for a button to revoke consent" hash = "sha1-b4524373ff63f37e062bd5f496e8ba04ee5c678d" other = "Widerrufen" [CertLoginIntroText] -hash = "sha1-e9f7c0522e49ffacc49e3fc35c6ffd31e495baf6" -other = "Die Anwendung {{ .ClientName }} fragt nach einer Anmeldung." +hash = "sha1-02871569c8d36e522cdb0716081ef62b7c7a71ec" +other = "Die Anwendung **{{ .ClientName }}** fragt nach einer Anmeldung." [CertLoginRequestText] hash = "sha1-1b20eea0f6fbb4ff139ecfe6b7a93c98cb14b8d7" @@ -92,12 +102,12 @@ hash = "sha1-00632c6562df53c62861c33e468e729887816419" other = "Besuche [die Freigabeverwaltung]({{ .ManageConsentHRef }}), um deine Freigaben für Applikationen einzusehen oder zu widerrufen." [IntroConsentMoreInformation] -hash = "sha1-f58b8378238bd433deef3c3e6b0b70d0fd0dd59e" -other = "Auf der Beschreibungsseite findest du mehr Informationen zu {{ .client }}." +hash = "sha1-836b2931b417e98db4102731604443ee2700123c" +other = "Auf der [Beschreibungsseite]({{ .clientLink }}) findest du mehr Informationen zu **{{ .client }}**." [IntroConsentRequested] -hash = "sha1-3ac6a3583d40b5e8930c57531f0be9706f1e0194" -other = "Die Anwendung {{ .client }} hat deine Zustimmung für die Erteilung der folgenden Berechtigungen angefragt:" +hash = "sha1-db5e67a16c181c4d27baf5d0e3bf255224b0fffc" +other = "Die Anwendung **{{ .client }}** hat deine Zustimmung für die Erteilung der folgenden Berechtigungen angefragt:" [LabelAcceptCertLogin] description = "Label for a button to accept certificate login" diff --git a/translations/active.en.toml b/translations/active.en.toml index 465e1d5..034ca40 100644 --- a/translations/active.en.toml +++ b/translations/active.en.toml @@ -1,6 +1,6 @@ AuthServerErrorExplanation = "A request that your browser sent to the authorization server caused an error. The authorization server returned details about the error that are printed below." AuthServerErrorTitle = "Authorization server returned an error" -CertLoginIntroText = "The application {{ .ClientName }} requests a login." +CertLoginIntroText = "The application **{{ .ClientName }}** requests a login." CertLoginRequestText = "Do you want to use the chosen identity from the certificate for authentication?" ClaimsInformation = "In addition the application wants access to the following information:" ConfirmRevokeExplanation = "Do you want to revoke your consent to allow **{{ .Application }}** access to identity data for **{{ .Subject }}**?" @@ -10,8 +10,8 @@ ErrorUnknown = "Unknown error" HintChooseAnIdentityForAuthentication = "Choose an identity 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." -IntroConsentRequested = "The {{ .client }} application requested your consent for the following set of permissions:" +IntroConsentMoreInformation = "You can find more information about **{{ .client }}** at [its description page]({{ .clientLink }})." +IntroConsentRequested = "The **{{ .client }}** application requested your consent for the following set of permissions:" LabelConsent = "I hereby agree that the application may get the requested permissions." LabelNever = "Never" LabelSubmit = "Submit" @@ -43,6 +43,14 @@ other = "Cancel" description = "Title for a button to confirm consent revocation" other = "Yes, Revoke!" +[ButtonTitleConsent] +description = "Title for a button to give consent" +other = "Consent" + +[ButtonTitleDeny] +description = "Title for a button to deny consent" +other = "Deny" + [ButtonTitleRevoke] description = "Title for a button to revoke consent" other = "Revoke" diff --git a/ui/templates/client_certificate.gohtml b/ui/templates/client_certificate.gohtml index c339ea7..8a7eb10 100644 --- a/ui/templates/client_certificate.gohtml +++ b/ui/templates/client_certificate.gohtml @@ -1,7 +1,7 @@ {{ define "content" }}

{{ .Title }}

-

{{ .IntroText }}

+ {{ .IntroText }} {{ with .FlashMessage }}