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

The First and Last name can be replaced with null when missing from Azure AD #19

Open
CamiAndrei opened this issue Nov 10, 2022 · 5 comments
Labels
Type: Bug Something isn't working

Comments

@CamiAndrei
Copy link

CamiAndrei commented Nov 10, 2022

In the Azure AD the login is done based on the email address.
In XWiki in case the email already exists a new account is not created but updated with details from Azure AD.
In the case when the First or Last name are missing from Azure AD they are replaced with null.
Since there is a synchronization, you can't edit the details in XWiki, because they get updated with null, since they don't exist in Azure.

We should avoid overwriting information with null.

@CamiAndrei CamiAndrei added the Type: Bug Something isn't working label Nov 10, 2022
@lucaa
Copy link

lucaa commented Nov 10, 2022

So, if I understand correctly, this is about making a "selective synchronization" of first name and last name, sometimes synchronising, sometimes preserving the value set in XWiki (from a previous sync or manually).

Whether we should do such a selective sync or not, for me, it really depends on what "missing from Azure AD" means, of the thing that we use as the "missing" semantic.

What we should definitely avoid is to make a judgement about what is "valid" and what is "not valid", from a value provided by Azure AD for the claims the authenticator makes. The risk with that is that this judgement may be correct for this situation and wrong for another one. For example, if we receive, from Azure, the string value "null" and we decide that we ignore that value, this would be an interpretation, on our side, that "null" is not a valid value and we can / should discard. I don't think it would be safe to go there and make such interpretations.

However, if the protocol contains any semantic for the server to 'refuse' a claim from the client or 'not support' an answer for a claim, this would be something that would be "safe" to interpret as "missing" and not do the synchronisation in that case.

@lucaa
Copy link

lucaa commented Nov 10, 2022

So, depending on which one of the 2 happens, how that information is "missing" from Azure, this may be a bug or a new feature.
If they're "missing" from Azure in a way that makes that the values exist in Azure but they have a wrong value, then this is not a bug.

@CamiAndrei
Copy link
Author

Hi!
So, if I understand correctly, this is about making a "selective synchronization" of first name and last name, sometimes synchronizing, sometimes preserving the value set in XWiki (from a previous sync or manually).
If by ignoring empty field in Azure, this would be selective synchronization, then yes, that was my though. But I definitely didn't meant to add the option to uncheck the Permission request or other option to selective synchronization based choice.

@CamiAndrei
Copy link
Author

how that information is "missing" from Azure,
It is not a mandatory information in Azure AD, so it can be left empty. However it mandatory in XWiki. Or at least it seems that XWiki expects to get some information from those fields, and when the information is not there adds null.

@lucaa
Copy link

lucaa commented Nov 10, 2022

It is not a mandatory information in Azure AD, so it can be left empty. However it mandatory in XWiki. Or at least it seems that XWiki expects to get some information from those fields, and when the information is not there adds null.

That is exactly the point of my comment, to make a distinction between Azure sending an empty value or Azure not sending any value at all or sending an answer meaning "I have no value for this". An empty value is a legitimate value in some cases, so it would be wrong for us to make the assumption that it should be ignored, I would say.

We'd need to debug this more to understand what happens when this null is added, whether it's a shortcut from XWiki ignoring a legitimate "I don't have this value" answer from Azure or whether it's Azure that sends a wrong value as an answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants