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.

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

Add trust level to attribute #52

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Weasy666
Copy link
Contributor

Add trust_level attribute to Attribute

@njaremko
Copy link
Owner

Context? trust_level doesn't seem to be a part of the standard

@Weasy666
Copy link
Contributor Author

Please excuse my late answer.
We have a few IdPs here in Germany which add such a TrustLevel attribute to the Attribute element in their responses. And we need to be able to access those. Adn the only way, i am aware of, to access them is to add them here.

@njaremko
Copy link
Owner

Please excuse my late answer. We have a few IdPs here in Germany which add such a TrustLevel attribute to the Attribute element in their responses. And we need to be able to access those. Adn the only way, i am aware of, to access them is to add them here.

I suspect many IdPs have custom properties like this. I can't merge a change like this that pollutes the struct for everyone to support a custom field for you.

I suspect proper support here probably requires writing an attribute trait and switching uses to the methods on that trait. That'd allow people working with IdPs like this to write custom structs with this library's Attribute struct nested inside for the standard attributes, the few custom attributes, and then using the serde flatten stuff.

Thoughts?

@Weasy666
Copy link
Contributor Author

That's fair. The "nested" approach seems reasonable. I'll change the PR to draft and see if i can find the time to whip something up

@Weasy666 Weasy666 marked this pull request as draft May 29, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants