Skip to content

Conversation

HichamDz38
Copy link

Values can be 0,1 empty string etc when stored in the database or in data conversion it's not required to be True/False specifically

Values can be 0,1 empty string etc when stored in the database or in data conversion it's not required to be  True/False specifically
@HichamDz38
Copy link
Author

i don't know why these tests fails, but i don't thing that its' because on the PR changes, might be it will fails with the actual code as it is as well

@pitbulk
Copy link
Contributor

pitbulk commented Mar 10, 2025

The possible values for requestedAuthnContext are True, False or a List. I have tests that check that those are the introduced values.

At the doc in the settings section you can read

// Authentication context.
// Set to false and no AuthContext will be sent in the AuthNRequest,
// Set true or don't present this parameter and you will get an AuthContext 'exact' 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport'
// Set an array with the possible auth context values: array ('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:X509'),

Why don't you instead force conversion from DB to the toolkit?

Your suggested code fails as a list will pass the condition:

if security["requestedAuthnContext"]

@HichamDz38
Copy link
Author

it's possible to do that in another way, by checking first if the security["requestedAuthnContext"] is a list by : isintance(security["requestedAuthnContext"],list),
it's a common for developers that True/False not required to be specifically/literally True or False it can be 0, 1 etc,
i can do that: checking if it's a list first and the tests will pass,
is that ok?

fix conditions
@pitbulk
Copy link
Contributor

pitbulk commented Mar 11, 2025

If you first check for the list, it should be ok

@HichamDz38
Copy link
Author

@pitbulk , i did that already in the last commit,
however the tests still fails,
by the way i run the tests in the master branch, and lot of them fails

@HichamDz38
Copy link
Author

@pitbulk , i am still waiting for your response,
is what i am proposing okey or i need to do any changes?
greetings

@pitbulk pitbulk changed the base branch from master to test_py3_12 July 3, 2025 15:53
@pitbulk pitbulk changed the base branch from test_py3_12 to master July 3, 2025 15:53
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