CASC-189 : Handle authentication date returned by custom CAS 2.0 service ticket validation #19
Conversation
…ice ticket validation
| private CommonUtils() { | ||
| // nothing to do | ||
| } | ||
|
|
||
| public static String formatForUtcTime(final Date date) { | ||
| final DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); | ||
| dateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); | ||
| final DateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT); |
There was a problem hiding this comment.
Refactor lines 78 and 79 into a common private method to be used by both methods. (this and the one below)
There was a problem hiding this comment.
Right, I use common constants but I can go further. I'll fix this.
|
I'm not a fan of this pull request. It preempts the protocol discussion on extending the protocol and would mean that either (a) we're forcing the protocol to take this without a discussion or (b) forcing the client to support both methods if this ends up not being the one in the protocol revision. |
|
I send you a direct email on this and ask the question on the mailing list : https://lists.wisc.edu/read/messages?id=18419320, but as I didn't get any answer, I tried to move the discussion on a pull request basis. We talked with Rob Winch to get a CAS remember-me support based on timeout and authentication creation date : spring-projects/spring-security#14. So that's why I created this pull request to try to make things go forward as I don't think it needs more discussion. I also foresee to post a message about this authentication date to the spec revision thread but I didn't get time to do it. Obviously I was wrong. What do you want me to do ? Open a new thread on mailing list ? Should I fix the pull request according to all remarks or give up on this ? |
|
Code issues aside I think this is a pretty reasonable ad hoc CAS protocol extension that provides something analogous to the SAML Assertion IssueInstant attribute. Maybe the protocol point merits further discussion, but I can imagine we'd end up here eventually. |
|
I think we'd end up with something similar also. However, since there is a protocol discussion going on, I don't think its right to put this in without the conversation happening within that thread. That said, if we can bang out a 2.01 protocol spec with this in it, let's do it :-) |
|
OK. I will share this on mailing list and see how it turns out... |
|
I raised that question in another commit as well: de we need a isValid() method on the assertion. Currently the Authentication filter is not checking any expiry time or other properties. It just checks the existence of a object. When the Interface of the assertion has a isValid() this would at least handle the cases where already an expiry (or refresh timer) is expired. |
|
Can you open a JIRA issue for the is valid request?
|
|
@battags Yes I will, need to do some research first. Thanks. Update: I did: https://issues.jasig.org/browse/CASC-191 |
|
I have corrected the pull request according to Misagh's and Scott's remarks... |
|
@battags : as the CAS server 4.0 will return the authentication date, I think it makes sense to merge this pull request, doesn't it ? |
|
Is the code in this change consistent with the emerging CAS 3.0 protocol spec? If yes, then I'm +1 to accept. |
|
Totally right, I checked : apereo/cas#224 and I don't see the |
|
@leleuj did we get authenticationDate added? |
|
Unfortunately no : apereo/cas#224. |
|
@leleuj can we close this for now then and open a JIRA issue (tracked against 3.4) to support this? |
|
Right, I targeted the JIRA CASC-189 for 3.4... |
To add CAS remember-me support in Spring Security (based on time between authentication creation date and now) : spring-projects/spring-security#14, we now handle authentication date from SAML response.
For CAS proxy service (as we don't have SAML validation), I propose to handle authentication date from CAS 2.0 service ticket validation response if the custom extra attribute cas:authentiationDate is used.