CAS-1283: principal attributes in the service validate success payload. #224
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <%-- | ||
|
|
||
| Licensed to Jasig under one or more contributor license | ||
| agreements. See the NOTICE file distributed with this work | ||
| for additional information regarding copyright ownership. | ||
| Jasig licenses this file to you under the Apache License, | ||
| Version 2.0 (the "License"); you may not use this file | ||
| except in compliance with the License. You may obtain a | ||
| copy of the License at the following location: | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, | ||
| software distributed under the License is distributed on an | ||
| "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| KIND, either express or implied. See the License for the | ||
| specific language governing permissions and limitations | ||
| under the License. | ||
|
|
||
| --%> | ||
| <%@ page session="false" contentType="text/plain" %> | ||
| <%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %> | ||
| <cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'> | ||
|
There was a problem hiding this comment. I think we talked about adding There was a problem hiding this comment. We did talk about. Any thoughts if this will cause problems with any of the clients? There was a problem hiding this comment. I tend to say no, but it's hard to tell without testing. For safety, we can post-pone this change for 4.0 revision... |
||
| <cas:authenticationFailure code='${code}'> | ||
| ${fn:escapeXml(description)} | ||
| </cas:authenticationFailure> | ||
| </cas:serviceResponse> | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <%-- | ||
|
|
||
| Licensed to Jasig under one or more contributor license | ||
| agreements. See the NOTICE file distributed with this work | ||
| for additional information regarding copyright ownership. | ||
| Jasig licenses this file to you under the Apache License, | ||
| Version 2.0 (the "License"); you may not use this file | ||
| except in compliance with the License. You may obtain a | ||
| copy of the License at the following location: | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, | ||
| software distributed under the License is distributed on an | ||
| "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| KIND, either express or implied. See the License for the | ||
| specific language governing permissions and limitations | ||
| under the License. | ||
|
|
||
| --%> | ||
| <%@ page session="false" %> | ||
| <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> | ||
| <%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %> | ||
| <cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'> | ||
|
There was a problem hiding this comment. Same comment about |
||
| <cas:authenticationSuccess> | ||
| <cas:user>${fn:escapeXml(assertion.chainedAuthentications[fn:length(assertion.chainedAuthentications)-1].principal.id)}</cas:user> | ||
|
There was a problem hiding this comment. The changes to Assertion in 31ec59d may afford some opportunity to clean up the JSTL here and in similar usage elsewhere in this file. There was a problem hiding this comment. Certainly, yes. Will keep that in mind after the merge. |
||
| <c:if test="${not empty pgtIou}"> | ||
| <cas:proxyGrantingTicket>${pgtIou}</cas:proxyGrantingTicket> | ||
| </c:if> | ||
| <c:if test="${fn:length(assertion.chainedAuthentications) > 1}"> | ||
| <cas:proxies> | ||
| <c:forEach var="proxy" items="${assertion.chainedAuthentications}" varStatus="loopStatus" begin="0" | ||
| end="${fn:length(assertion.chainedAuthentications)-2}" step="1"> | ||
| <cas:proxy>${fn:escapeXml(proxy.principal.id)}</cas:proxy> | ||
| </c:forEach> | ||
| </cas:proxies> | ||
| </c:if> | ||
|
|
||
| <c:if test="${fn:length(assertion.chainedAuthentications[fn:length(assertion.chainedAuthentications)-1].principal.attributes) > 0}"> | ||
| <cas:attributes> | ||
| <c:forEach var="attr" | ||
| items="${assertion.chainedAuthentications[fn:length(assertion.chainedAuthentications)-1].principal.attributes}" | ||
| varStatus="loopStatus" begin="0" | ||
| end="${fn:length(assertion.chainedAuthentications[fn:length(assertion.chainedAuthentications)-1].principal.attributes)}" | ||
| step="1"> | ||
| <cas:${fn:escapeXml(attr.key)}>${fn:escapeXml(attr.value)}</cas:${fn:escapeXml(attr.key)}> | ||
| </c:forEach> | ||
| </cas:attributes> | ||
| </c:if> | ||
|
|
||
| </cas:authenticationSuccess> | ||
| </cas:serviceResponse> | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we keep the old responses ? Are they used somewhere ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reason version 1 is still there, no? There are A LOT of talks about "backwards compatibility", etc. so I thought to leave v2 in there in case someone, somewhere would require it, or v3 (default) would break some clients, etc., so this would be an easy "escape route" to plug views for older versions back in.
Same reason the "protocols" directory structure exists for views in the first place, so multiple "protocols" could be used. No? Have I misunderstood?
If not, then i suggest to remove ALL the older protocols and keep the only one, latest protocol view.
Cheers,
D.
—
Sent from Mailbox for iPhone
On Tue, Apr 9, 2013 at 6:12 AM, LELEU Jérôme notifications@github.com
wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the version 1 will be still available through the /validate url, which is not the case of the 2.0, replaced by the 3.0 on /serviceValidate and /proxyValidate urls.
That's why I would prefer cleaning than keeping for backward compatibility.
Views defined in the protocol/ directory are also used when you enable the appropriate protocol : OpenID, OAuth, ClearPass, aren't they ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, what's your suggestion? Delete protocols/2.0 and just leave protocols/3.0 directory with JSPs there?
—
Sent from Mailbox for iPhone
On Tue, Apr 9, 2013 at 6:38 AM, LELEU Jérôme notifications@github.com
wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I would just keep
protocol/3.0instead ofprotocol/2.0...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its valid to remove 2.0. There is a 2.0 protocol, and those files support it explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that was my intention in the first place. I will not do any more work on this until (and if) consensus on what is the right thing to do here is reached among CAS committers :-)
Cheers,
D.
—
Sent from Mailbox for iPhone
On Tue, Apr 9, 2013 at 7:44 AM, Scott notifications@github.com wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3.0 protocol is meant to supersede 2.0 in way that 2.0 didn't for 1.0. That is, 3.0 is meant to replace the current response at the current 2.0 endpoints (since we're just documenting common practice) rather than inventing a new validation response. This seems to argue in favor of removing the older 2.0 views.
That said, I see no harm in leaving them where they are, and they do provide potential benefit to deployers, as Dima suggests, of being able to fall back to 2.0 if that became necessary for some reason.
+1 for commit.