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_Syste...
https://access.redhat.com/site/documentation/en-US/Red_Hat_Certificate_Syste...
https://access.redhat.com/site/documentation/en-US/Red_Hat_Certificate_Syste...
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).