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

ARP being ran twice #632

Closed
tvdijen opened this issue Feb 7, 2019 · 8 comments · Fixed by #1139
Closed

ARP being ran twice #632

tvdijen opened this issue Feb 7, 2019 · 8 comments · Fixed by #1139

Comments

@tvdijen
Copy link
Contributor

tvdijen commented Feb 7, 2019

According to this documentation, the processing order is:
first ARP, then AM.

It seems however, ARP is being ran after AM for the second time. Logging seems to confirm this as well (see first + last line):

[2019-02-07 10:01:06] engineblock.INFO: Applying attribute release policy for https://acc-anvs.mavimcloud.com/saml2 {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.NOTICE: AssertionConsumerServiceLocation 'https://acc-anvs.mavimcloud.com/Saml2/Acs' or ProtocolBinding '' were mentioned in request, but not both! Ignoring... {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.NOTICE: AssertionConsumerServiceLocation 'https://acc-anvs.mavimcloud.com/Saml2/Acs' or ProtocolBinding '' were mentioned in request, but not both! Ignoring... {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.INFO: Using internal binding for destination https://engine.sson.accsscict.rijksweb.nl/authentication/idp/provide-consent {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} {"url_params":{"EntityCode":"main","ServiceName":"provideConsentService","RemoteIdPMd5Hash":""}} [2019-02-07 10:01:06] engineblock.INFO: Calling service 'provideConsentService' {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.INFO: Using internal binding for destination https://engine.sson.accsscict.rijksweb.nl/authentication/proxy/processed-assertion {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} {"url_params":{"EntityCode":"main","ServiceName":"processedAssertionConsumerService","RemoteIdPMd5Hash":""}} [2019-02-07 10:01:06] engineblock.INFO: Calling service 'processedAssertionConsumerService' {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.NOTICE: AssertionConsumerServiceLocation 'https://acc-anvs.mavimcloud.com/Saml2/Acs' or ProtocolBinding '' were mentioned in request, but not both! Ignoring... {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] [2019-02-07 10:01:06] engineblock.NOTICE: AssertionConsumerServiceLocation 'https://acc-anvs.mavimcloud.com/Saml2/Acs' or ProtocolBinding '' were mentioned in request, but not both! Ignoring... {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} [] EBLOG[5882]: [2019-02-07 10:01:06] engineblock.INFO: Applying attribute release policy for https://acc-anvs.mavimcloud.com/saml2 {"session_id":"ea5utk4d5iskv2v94l440ofe05","request_id":"5c5bf3d294feb"} []

Consider the following scenario for reproduction:

  • We receive attributes A and B from an IDP.
  • We release attributes A and B in ARP
  • We use AM to map attributes A > C and B > D and then unset A and B
  • Expected result is attributes C and D being sent to the SP. However, none are sent.

When ARP is set to release A, B, C and D, it works as expected.

Unrelated, but also note the NOTICEs (also twice?). This particular SP is only sending an ACS-location, without protocolBinding. This seems to be a perfectly valid situation according to SAML2int

@thijskh
Copy link
Member

thijskh commented Feb 7, 2019

Yes, the ARP is applied twice, since the start. This should be updated in the documentation.

As for the unrelated note, this is fixed by #611 I think.

@tvdijen
Copy link
Contributor Author

tvdijen commented Feb 7, 2019

Is there a reason why? It doesn't make much sense to me..
It means I have to introduce all the mapped attributes in Manage as well, even though they are very specific for just one SP..

@thijskh
Copy link
Member

thijskh commented Feb 7, 2019

It is currently the way how eduPersonTargetedID works.

A solution is to disable the ARP and ensure the AM only outputs the desired attributes.

@tvdijen
Copy link
Contributor Author

tvdijen commented Feb 7, 2019

Thanks! I had thought of that myself, but I see it as a workaround rather than a solution.
I think the solution could be:

  • Set eduPersonTargetedID
  • Run ARP + AM
  • Update value of eduPersonTargetedID if it still exists at this point.

I think it would be fairly easy to implement this..
Do you see any issues with the proposed solution and would you consider a PR for this if I sent it in?

@thijskh
Copy link
Member

thijskh commented Feb 4, 2020

Yes. Although I’m not sure I yet oversee all implications. But a concrete proposal may help there.

@thijskh
Copy link
Member

thijskh commented May 27, 2021

We are also running into some issues with the ARP being ran twice.

Most notably: need to disable the ARP completely if doing an AM (otherwise ARP will filter newly created attributes in AM) and ARP with AA will overwrite any changes made in AM to AA sourced attributes.

Did you get any further with this already? We also have an interest to improve this.

@thijskh
Copy link
Member

thijskh commented May 31, 2021

@tvdijen I made a proposal in the linked PR. Maybe you can see if this is useful for you as well?

@tvdijen
Copy link
Contributor Author

tvdijen commented May 31, 2021

Works for me! The first and third bullet were the two reasons for me to open this issue..
Also, sorry, I somehow missed the previous message....

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 a pull request may close this issue.

2 participants