bnfinet
commented
4 years ago Thanks for the suggestion and contribution.
Since you have code ready to go (and tests! Thank you), please feel free to open a PR and we can continue the conversation mostly here.
FYI - I'm running behind on reviews but hope to have time next week.
On Fri, Oct 23, 2020, 1:19 PM David Manu notifications@github.com wrote:
Contributors
This issue and any associated commits/PRs are contributions are made on behalf of the following individuals while employed on contract with IBM Canada. Contact David Manu https://github.com/davmanu for any clarifications.
- Darius Asri Rezaei https://github.com/dasrirez
- Chris Staykov https://github.com/cstaykov
- Omar Alashqar https://github.com/omaralashqar
Proposal
This feature enables optional & configurable case-insensitivity on all or a specified set of email domains in the whitelist.
note: The implementation of this proposed features is complete, and includes full test coverage. Background
In the event an identity provider sends a user's email containing capitalization that does not explicitly match the Vouch whitelist the result is an unexpected denial of access.
While it's true that the responsibility should fall on the identity providers to send Vouch the correct capitalization, and for Vouch to be configured with a matching value, this feature would nevertheless be useful when:
- The identity provider may provide unknown or inconsistent capitalization
- The usernames' capitalization is not known at configuration time (bulk add into whitelist)
For implementations containing only a few users (and therefore only a few email addresses), it would be reasonable to add multiple capitalization permutations to the whitelist. However, this becomes troublesome and error-prone as the whitelist grows. Other solution approaches could would provide better, or more consistent means to provide whitelist values to Vouch, but the addition of this feature allows for quick implementations while remaining entirely within Vouch.
As per RFC 2821 https://tools.ietf.org/html/rfc2821, the case-sensitivity of an email is left to the discretion of the email server itself. We propose the ability to enable this feature globally or on a per-domain basis. Changes
Minor changes to:
- auth.go
- cfg.go
- handlers_test.go
Two new configuration parameters:
-
vouch.case_insensitive_emails: boolean Global setting to enable case-insensitivity for emails on all domains, as relevant in the whitelist. Set to false by default, which preserves current behaviour.
vouch.case_insensitive_email_domains: string[] If the global setting is not enabled, emails in the whitelist with these specific domains will be treated as case-insensitive. Set to an empty list by default, which preserves current behaviour.
Notes
- Emails with subdomains are treated independently since they might be provided by a different directory service
- This feature is extendable to other usage patterns (e.g. regex matching the whitelist for testing)
This issue was discovered in our implementation of Vouch (❤️ love the great work!) when using nginx for an internal non-commercial project and we have a fork that fully implements this feature. The code changes conform to the existing structure and were unit and integration tested. If you're interested in adding this feature, we can open a PR immediately.
Have a great day!
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vouch/vouch-proxy/issues/324, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJUV24KVPCNEVSITUJ2PPTSMHQORANCNFSM4S472VCA .
davmanu
Contributors
This issue and any associated commits/PRs are contributions are made on behalf of the following individuals while employed on contract with IBM Canada. Contact David Manu for any clarifications.
Proposal
This feature enables optional & configurable case-insensitivity on all or a specified set of email domains in the
whitelist.note: The implementation of this proposed features is complete, and includes full test coverage.
Background
In the event an identity provider sends a user's email containing capitalization that does not explicitly match the Vouch whitelist the result is an unexpected denial of access.
While it's true that the responsibility should fall on the identity providers to send Vouch the correct capitalization, and for Vouch to be configured with a matching value, this feature would nevertheless be useful when:
For implementations containing only a few users (and therefore only a few email addresses), it would be reasonable to add multiple capitalization permutations to the whitelist. However, this becomes troublesome and error-prone as the whitelist grows. Other solution approaches could would provide better, or more consistent means to provide whitelist values to Vouch, but the addition of this feature allows for quick implementations while remaining entirely within Vouch.
As per RFC 2821, the case-sensitivity of an email is left to the discretion of the email server itself. We propose the ability to enable this feature globally or on a per-domain basis.
Changes
Minor changes to:
Two new configuration parameters:
vouch.case_insensitive_emails: boolean\ Global setting to enable case-insensitivity for emails on all domains, as relevant in thewhitelist. Set tofalseby default, which preserves current behaviour.vouch.case_insensitive_email_domains: string[]\ If the global setting is not enabled, emails in thewhitelistwith these specific domains will be treated as case-insensitive. Set to an empty list by default, which preserves current behaviour.Notes
whitelistfor testing)This issue was discovered in our implementation of Vouch (:heart: love the great work!) when using nginx for an internal non-commercial project and we have a fork that fully implements this feature. The code changes conform to the existing structure and were unit and integration tested. If you're interested in adding this feature, we can open a PR immediately.
Have a great day!