Skip to content

entityID change affects saml:SP too#1710

Merged
tvdijen merged 7 commits into
simplesamlphp:masterfrom
ghalse:patch-entityid-refs
Oct 26, 2022
Merged

entityID change affects saml:SP too#1710
tvdijen merged 7 commits into
simplesamlphp:masterfrom
ghalse:patch-entityid-refs

Conversation

@ghalse
Copy link
Copy Markdown
Contributor

@ghalse ghalse commented Oct 25, 2022

Update the examples and documentation for saml:SP to be consistent with the notion that entityIDs are no longer
dynamically generated if the 'entityID' config option is null.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 25, 2022

Codecov Report

Merging #1710 (b3ac41d) into master (68d9f3b) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1710      +/-   ##
============================================
+ Coverage     42.55%   42.59%   +0.03%     
- Complexity     2205     2206       +1     
============================================
  Files            83       83              
  Lines          6194     6196       +2     
============================================
+ Hits           2636     2639       +3     
+ Misses         3558     3557       -1     

@tvdijen
Copy link
Copy Markdown
Member

tvdijen commented Oct 25, 2022

We've discussed this internally and we would like to throw an exception when people do not change the default entityID. You think you can add that to this PR?

@ghalse
Copy link
Copy Markdown
Contributor Author

ghalse commented Oct 26, 2022

We've discussed this internally and we would like to throw an exception when people do not change the default entityID. You think you can add that to this PR?

Done.

It wasn't clear if you wanted to do that for the example in metadata-templates/saml20-idp-hosted.php as well, so I've added that as a separate commit. Feel free to leave that out if you don't want to use that :)

@tvdijen
Copy link
Copy Markdown
Member

tvdijen commented Oct 26, 2022

Yes, that was what I had in mind, but I lacked the time to make this PR myself! Thanks a lot!!

@tvdijen tvdijen merged commit d8f1f5d into simplesamlphp:master Oct 26, 2022
tvdijen added a commit that referenced this pull request Oct 26, 2022
* entityID change affects saml:SP too

* Generate exception for example entityID (SP)

* Make entityId consistent with examples

* Generate exception for example entityID (IdP)

* Add some use-statements

Co-authored-by: Tim van Dijen <tvdijen@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

\n This pull request has been automatically locked since there has \n not been any recent activity after it was closed.\n Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants