Now allowing Authsources to set validUntil and cacheDuration for Metadata. - #1481
Now allowing Authsources to set validUntil and cacheDuration for Metadata.#1481tommy2d wants to merge 6 commits into
Conversation
Sorry, something went wrong.
thijskh
left a comment
There was a problem hiding this comment.
Needs to be changed according to the (later) changes to the SP metadata refactoring + unit test update so we keep coverage of the new code up to date.
Sorry, something went wrong.
| if ($metadata['expire'] - time() < $this->maxDuration) { | ||
| $this->maxDuration = $metadata['expire'] - time(); | ||
| if ($metadata['expire'] - time() < $this->cacheDuration) { | ||
| $this->cacheDuration = $metadata['expire'] - time(); |
There was a problem hiding this comment.
I don't understand what this code wants to achieve. There is an expiration in the metadata. So we whould likely set validUntil to that value I think? Not cacheDuration? In any case the docs should probably say how the option interacts with expire.
Sorry, something went wrong.
There was a problem hiding this comment.
Agreed.. I think it's also the wrong setting being set here.. Our metadata-converter converts validUntil to expire.
I think it makes sense to drop the expire-option in favour of validUntil.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review. @@ Coverage Diff @@
## master #1481 +/- ##
=========================================
Coverage 40.05% 40.05%
- Complexity 3531 3532 +1
=========================================
Files 141 141
Lines 10596 10598 +2
=========================================
+ Hits 4244 4245 +1
- Misses 6352 6353 +1
|
Sorry, something went wrong.
|
I've rebased against master because the PR targeted the 1.19-branch and this is not something we can merge there. I've made a backup of the branch before I did. |
Sorry, something went wrong.
|
There's still quite a lot of code that refers to expire when talking about remote entities I think. Are you referring to that or to the setting for hosted entities (does that exist?). |
Sorry, something went wrong.
|
It does not exist for hosted entities I think, so for remote entities... validUntil is translated to the expire setting by our metadata converter and I think it makes sense to rename it to validUntil.. We have to update the metarefresh-module as well if we do so |
Sorry, something went wrong.
Now allowing generation of SP metadata XML that includes @validuntil or @cacheDuration at EntityDescriptor.