<div dir="ltr">Hi Mark, <div><br></div><div>Thanks for your comments, replies below...<div><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 22, 2018 at 5:01 AM Mark Taylor <<a href="mailto:M.B.Taylor@bristol.ac.uk">M.B.Taylor@bristol.ac.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Brian,<br>
<br>
On Fri, 2 Nov 2018, Brian Major wrote:<br><br>
First, the the "search API" defines two functions:<br>
<br>
boolean isMember(Group, User)<br>
list<Group> getMemberships(User)<br>
<br>
and these are implemented by the REST API under the endpoints:<br>
<br>
GET /search/{group}<br>
GET /search<br>
<br>
Unless I'm misunderstanding what's going on here (possible),<br>
it seems a bit unnecessarily confusing to give these completely<br>
different names in the abstract API and the REST interface.<br></blockquote><div><br></div><div>Right. The first set of 'functions' was really just a way for me to work through what features are needed by the search API. The second set is the translation of those features to REST. This obviously came across as confusing so perhaps the first set of features should be removed... ?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Would it be better to homogenise them a bit, e.g.<br>
<br>
GET /gms/isMember/{group}<br>
GET /gms/getMemberships<br></blockquote><div><br></div><div>I see what you're saying but a homogenized version doesn't seem RESTful. 'search' in the path is intended to identify a resource, but this is a stretch because 'search' is really more of an operation. Also, 'search' serves as a way of namespacing the API. I can see GMS supporting other capabilities at some point. For example, to manage groups and membership, you may have something like 'GET /groups/{group}. Anyhow, I'm certainly open to suggestions on how this API should be denoted. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Second, the way in which HTTP return codes are used to communicate<br>
results seems slightly fishy.<br>
<br>
The text says:<br>
<br>
"An HTTP GET to /search/{group} shall respond with HTTP 200 (OK) if<br>
the user is a member of the {group}. If the user is not a member of<br>
the group, or if the user is not recognized, or if the group is not<br>
recognized, the service shall respond with HTTP 403 (Forbidden)."<br>
<br>
This looks funny to me, since the request is to find out whether<br>
a given user is a member of a named group, and a positive or<br>
negative answer is being given - in the case of no membership,<br>
the request about membership is not being forbidden (403) it's<br>
just being answered in the negative. The forbidden-ness doesn't<br>
really apply to this response, just to the responses that a user<br>
with the given identity might expect if s/he tries to do something<br>
auth-sensitive in the future.<br></blockquote><div><br></div><div>Yes, I agree--this is not using HTTP response codes as intended. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Moreover, RFC2616 section 10.4.4 says this about the 403 Forbidden<br>
response code:<br>
<br>
"The server understood the request, but is refusing to fulfill it.<br>
Authorization will not help and the request SHOULD NOT be repeated."<br>
<br>
But in the case of this endpoint, authorization might help, since<br>
in the case where the request specifies the queried user's identity<br>
implicitly (i.e. no user/principal request parameters supplied),<br>
resubmitting the request with different credentials would<br>
constitute a query about a different user, so might very well<br>
give a different result.<br></blockquote><div><br></div><div>Another good point.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
So, maybe communicating the result using the (text/plain?) payload<br>
of a 200 response might be more appropriate than communicating<br>
the result using the HTTP response code.<br></blockquote><div><br></div><div>Okay, so I agree it needs to change. I think we have a number of alternatives (for a GET to /search/{group})</div><div><br></div><div>1. For non error situations, always return HTTP 200 OK.</div><div><br></div><div> Variant A: text/plain response: Return the single group name if the user is a member, return an empty document if not. (This is consistent with the GET /search behaviour.)</div><div><br></div><div> Variant B: text/plain response with 'true' if the user is a member and 'false' if the user is not a member.</div><div><br></div><div> Variant C: Return an XML document with an element that describes the group. This is like Variant A but would allow the response to be extended by evolving the XML schema if we find there is more information about the group that needs to be conveyed. With this approach we'd apply this pattern to GET to /search as well.</div><div><br></div><div>2. If the user is a member of the group, return HTTP 200 OK. If the user is not a member of the group, return HTTP 204 No Content. Maybe this is a violation of the HTTP rules too. In its definition, this line I found interesting:<br><br> "This response is primarily intended to allow input for actions to take place without causing a change to the user agent's active document view, although any new or updated metainformation SHOULD be applied to the document currently in the user agent's active view."</div><div><br></div><div>Cheers,</div><div>Brian</div></div></div></div></div>