Conversation
Codecov Report
@@ Coverage Diff @@
## master #1658 +/- ##
============================================
- Coverage 41.72% 41.71% -0.01%
+ Complexity 3660 3656 -4
============================================
Files 154 154
Lines 10721 10720 -1
============================================
- Hits 4473 4472 -1
Misses 6248 6248 |
thijskh
left a comment
There was a problem hiding this comment.
I think it's a great thing to make people choose an explicit entityID. That will surely prevent issues in the future.
Some comments:
- I'm missing changes to the SP auth source. Is this purposefully only done for IdP entityID generation?
- I would want to add some guidance for people on what to pick/what a good value is. It's not obvious and you cannot really change it afterwards.
- Probably the default suggestion should follow this advice. Which I think is not something that's an URN.
|
"You need to set an entityID: a string that uniquely identifies this IdP or SP. It must be a URL or URN. We recommend to choose something that is likely to remain stable over time. Examples (replace example.com with your organization's domain name: |
|
If that's going to be our advice, we might as well continue to auto-generate it.. "An entityID should be chosen in a manner that minimizes the likelihood of it changing for political or technical reasons, including for example a change to a different software implementation or hosting provider." It shouldn't hold an organization's domain name for that reason. |
|
I'm totally in favour of giving explicit advice on how to generate a good entity ID. I'm also against automatically generating it, since we don't have absolutely any context to provide an identifier that's independent of the particular configuration / deployment, and as such, it is likely to be problematic in the future if anything in the deployment changes. Only the persons installing the software knows how they plan to use it, and as such, only they know what's a good unique identifier for it. Regarding the discussion between URLs and URNs... is there any good practice or recommendations against using URNs? I'm not aware of any, and I think they have two immediate benefits:
|
|
Let's separate out the discussion on what the advice should be. |
dd0a693 to
907d838
Compare
Co-authored-by: Thijs Kinkhorst <thijs@kinkhorst.com>
Co-authored-by: Thijs Kinkhorst <thijs@kinkhorst.com>
|
I think all of the comments have been addressed and taken care of. Only thing left is the instructions to set a good entityID |
thijskh
left a comment
There was a problem hiding this comment.
I think only thing still missing is an update to simplesamlphp-sp.md. Other than that, good to go. Let's discuss entityID strategy in a separate issue.
|
\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. |
We used to auto-generate entity IDs pointing to the metadata-url, but that goes against SAML2INT recommendations.
Instead, we let users set the entityID themselves.
Closes point 4 in #1643