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

Correctly parse Attributes with multiple AttributeValues as arrays of strings #27

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

craigdrayton
Copy link

Implemented fix for #26

If one AttributeValue exists, it is returned as a string (as per current behaviour).
If more than one AttributeValue exists, they are returned as an array of strings.

Copy link
Owner

@steffow steffow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added yr changes manually as there were conflicts (and yr code is just a few lines). Wanna rebase and try to push?

if (Meteor.settings.debug) {
console.log("Name: " + attributes[i]);
console.log(`Adding attrinute from SAML response to profile:` + attributes[i].getAttribute('Name') + " = " + value.textContent);
console.log(`Adding attribute from SAML response to profile:` + attributes[i].getAttribute('Name') + " = " + value);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you're using 'value' vs 'value.textContent'?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of multiple attribute values, where "value" is an array, "value.textContent" will be undefined. Logging "value" should work whether it is a string or an array.

@craigdrayton
Copy link
Author

It may be simpler to keep your manual update and discard the PR :-)

@atao60
Copy link

atao60 commented Jan 13, 2019

@steffow thanks for this package.
It seems to me that not all the changes of PR #27 have been made in the current code (v0.0.17).
Then it's not working as it.
I had to change the lines 464 and 466 (v0.0.17) from:

console.log(`Adding attrinute from SAML response to profile:` + attributes[i].getAttribute('Name') + " = " + value.textContent);
profile[attributes[i].getAttribute('Name')] = value.textContent;

to:

console.log(`Adding attribute from SAML response to profile:` + attributes[i].getAttribute('Name') + " = " + value);
profile[attributes[i].getAttribute('Name')] = value;

@steffow
Copy link
Owner

steffow commented Jan 15, 2019 via email

@atao60
Copy link

atao60 commented Jan 16, 2019

Do you have an example to see the difference?

@steffow what do you need? Some code in a github repo?

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.

3 participants