On Thu, 2013-06-27 at 13:23 -0500, Endi Sukma Dewata wrote:
On 6/27/2013 10:20 AM, Ade Lee wrote:
> 1. One of the decisions you made was to allow admins, agents and
> operators to access the same resources. So, unlike the rest of the java
> subsystems, we will have /tps/rest/tokens as opposed
> to /tps/rest/agent/tokens or /tps/rest/admin/tokens
>
> One reason one might want to add /agent or /admin is if the content of
> GET /tps/tokens/X is different for admins/agents/operators.
>
> So what is the difference between op=search and op=search_admin, op=view
> and op=view_admin, op=show and op=show_admin? etc.
I'm still rebuilding my TPS environment so I can't check this myself
right now, but here are the screenshots from the docs and they look
pretty much identical:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Certificate_Sy...
https://access.redhat.com/site/documentation/en-US/Red_Hat_Certificate_Sy...
https://access.redhat.com/site/documentation/en-US/Red_Hat_Certificate_Sy...
> Is there any case where those differences need be preserved?
Even if there are differences, since they are the same resources (i.e.
tokens) most of the visible attributes will be the same. If there are
differences between the roles, it will be handled by returning the
common attributes plus the attributes visible to that role only. When
creating the REST data structure we combine all possible attributes.
I agree that there are more likely than not insufficient differences
between the representations of the resources for each role.
Part of the reason we chose to keep the calls separate for the other
subsystems is because of the different authentication method for each
interface. Agent and admin required client auth, ee required no auth.
This is not true for the TPS UI - all intefaces require client auth or
otherwise.
> 2. You use PUT /tps/rest/tokens/X as the operation to modify
tokens.
> Usually REST semantics imply that the resource passed in to the PUT
> operation essentially replace the resource at the server.
>
> Is that your intention? I'm guessing that the resource that would be
> passed in is the token object returned in GET /tps/rest/tokens/foo.
> It would help if you defined what a token resource was.
Any REST resource is a partial representation of the actual
object/service running on the server, meaning it may not have the full
information to reconstruct the object on the server-side. So even if we
PUT a resource it should be clear that we're not completely replacing
the actual object on the server, but we're updating the resource with
the attributes provided in the request.
In case of token resource, the resource consists of the attributes
returned by the GET operation which may not include everything a token
has. When modifying the resource using PUT some of these attributes will
be ignored because they are read-only (e.g. date created/modified), but
we can still PUT a resource that we obtained earlier using GET back to
the server to update some attributes.
Agreed.
> Or is your intention to send in something with an
"action" parameter -
> in which case, the correct operation is a POST?
I use 'action' in the modify token operation because I couldn't find a
better replacement for the 'question' parameter in the original
op=do_token. Ideally it should be called 'status' but there's already
another attribute with that name. Any suggestion? This 'action' should
actually be returned by the GET operation too. I'll add it after we
finalize the name.
do_token?question=X is essentially an operation where you are attempting
to set the state of the token resource - for example, from "active" to
"lost". There is no need for an additional parameter.
> 3. Should we be worried about a XSS attack here for modifying the state
> of the token? My guess is that we need to take advantage of the nonce
> mechanism, in which case, token state modification will require two
> steps.
Yes, but similar to our discussion in the past, nonces or ETags can be
added in a later phase without changing the interface. Since the
modification is a PUT operation, and the request attributes are obtained
using a GET operation done earlier, this is a two-step operation.
Without the nonce or ETag the GET operation will be optional, and the
client can construct the request from scratch. But later the PUT
operation can require a nonce or ETag from a previous GET operation.
OK, please put the nonce in the design. We dont want to forget it.
> 4. You should also note that we will be sending back BAD_REQUEST
(401?)
> when the token state transition is not permitted.
That should fall under "Normal application errors will return HTTP 4xx
return code." We can add more details as we implement it.
Right, I'm just pointing this out because in this case, there is a state
machine which determines which operations are successful. From the
point of view of the API, doing a token transition is just an attempt to
set the state of the token differently, but unlike something simple like
just changing an attribute value, the operation is subject to the
results of a state machine.
> 5. In the response to PUT /tokens/X, it is not necessary to
return the
> state of the token. Rather, you should return a URL pointer to the
> newly modified resource. The same comment applies to the other PUT
> operations as well.
Since PUT operation in general doesn't create a new resource, and also
the request is sent to the resource URL being modified, there is no need
to return a new location because it will be identical. In this design
the new location will only be returned on POST operations when adding a
new resource, which in this case the new resource location will be
different from the POST target (the resource collection).
Agreed. But in PUT /tokens/X, you return state information. This is
not necessary. You should probably return nothing - other than status.
> 6. Same question about XSS for add/remove token/ add/remove
user.
Similarly, nonces or ETags can be added later. As described earlier,
modifying users is a two-step process too with GET and PUT. Add and
remove operations do not require the client to do any other operation,
but it can be made so without changing the interface. The DELETE can
require a GET, but I'm not sure what operation should be required before
add.
Yeah - we need to think about ADD. Being able to add a privileged user
for example by XSS would be troubling.
> 7. Note that the difference between op=view_activity,
> op=view_activity_all etc. is in the types of activities that are
> visible. This should be handled seamlessly in the logic used to select
> activity records from the db.
Yes, the interface will be the same, but depending on which roles you
belong to you may get different results.
> 8. For the configuration items - audit config, profiles, profile
> mappings, connections, authenticators, I wonder if it makes things
> clearer to put them under a config bucket .. like /tps/rest/config/audit
> for example.
I'd rather not do that. For now I'm using /tps/rest/config to store
general settings. If there are resource-specific settings they should go
to that resource subtree.
For example, suppose later we want to provide interface to view audit
logs, it will be stored under /tps/rest/audit/logs. The audit config can
be stored in /tps/rest/audit, or maybe /tps/rest/audit/config. Imagine
you're an admin looking at a web page showing the audit logs. If you
want to change the audit settings you'd want to be able to click a
button on that page directly. Having to go to config -> audit menu will
be less intuitive. Not that this is relevant to determine the actual
resource URL, but with the same principles the config should be located
near the entries of that resource. It's also possible to provide two
interfaces at both locations for the same resources, but there's no real
need to do that now.
For the other resources mentioned above, they are both the entries and
the config. If we put them under config there wouldn't many (or any)
things left at the top level. Even system users can also be considered a
config, but unless there's a strong reason I prefer to keep them where
they are now.
OK.
> 9. Is there a mapping between Profile and Profile Mapping
operations and
> the old operations? Please put that in so we can see whether we have
> complete coverage. In particular, I do not see an operation to
> approve/enable a profile. This is important because we have Common
> Criteria requirements that two users are involved in the approval of a
> profile. In our case, IIRC we implemented it such that an admin can
> configure a profile but an agent must approve it.
There should be, but they are not in the docs and I have not had a
chance to test all possible operations in the UI. Once I get the TPS
instance back running I'll continue this. But in general we can
implement something similar to the cert requests:
/tps/rest/profiles/{id}/approve (all approvers will use the same URL)
/tps/rest/profiles/{id}/enable
Thats fine - lets make sure we get those operations in.
> 10. In POST /tps/rest/profilemappings, you return the location
of the
> profile mappings as well as the contents of the profile mapping
> resource. You should just return the location. In fact, its probably
> better to just return locations in general. The client would then use
> GET to fetch the details if needed. This comment applies to many of the
> resources.
In general I'd rather return the resource attributes in the POST
response than requiring the client to do a separate GET later. This
response acts as a receipt/acknowledgement for the add request, and it
will return the actual attributes stored on the server before any other
operation is done on it. This is rather important because sometimes a
lazy UI won't do an extra GET because it assumes the attributes don't
change, where in fact they could be normalized, ignored, or generated by
the server. It also prevents a possible (although unlikely) attack that
inserts an operation between the POST and GET.
I guess you are suggesting the same for PUT requests as above.
I'm not sure whether this is vaid reasoning or not. Just because you
see what has been posted (or PUT) in the server does not mean that this
data has not been changed the next time the data is accessed.
If anything, returning this data encourages lazy clients. The fact is
though that we will have to implement nonces and possibly etags, and so
GETs will become mandatory for any changes.
> 11. In the PUT /tps/rest/config, we have requirements for having
than
> one user to approve changes. Admins make changes and agents approve
> them if I recall correctly. You should look at the differences between
> the agent and admin pages.
I'll take a look again after I have the TPS instance back. Is there any
documents or diagrams showing the workflow of all approval processes in
TPS UI? Thanks.
The CC docs are probably the best bet for that (other than looking at
the code).