paperclip icon indicating copy to clipboard operation
paperclip copied to clipboard

Transform api operations based upon middleware wrappers

Open platy opened this issue 4 years ago • 4 comments

I came across the need to modify the api spec based on properties of middleware transformers. In our case it was RBAC middleware which specified different roles for different endpoints.

This change adds another trait to the parameter on the Resource wrapper, TransformApi. In this use case it allowed the roles required on the endpoints to be automatically added to operation.tags and into operation.description, which seems like it will be very useful.

I have a couple of concerns about it though:

  • this changes the signature of wrap so it is a breaking change for users of that function - it could be given a new name
  • I've made RouteWrapper public - maybe it should use a different type?
  • the transformers need to be cloned to be passed into the inner and kept in the wrapper - but in my case they are very small
  • the transformers are being kept in the wrapper in a type-system linked list, this avoided allocations and is fine in my case but maybe others would have a problem with it

platy avatar Nov 27 '20 09:11 platy

BTW, would not the roles required on endpoint better managed with Apiv2Security derive? We are managing this internally via FromRequest handlers on a User and AccessToken, that seems like better fit in actix world. Not sure about your use case details though.

In regards to back compatibility - we are doing 2 phase initialization of actix-web, first is for all services which we exclude from schema, then doing wrap_api and all schema based initializations. I guess this can be adviced as a migration approach if your PR would be accepted.

dunnock avatar Nov 27 '20 09:11 dunnock

Yes I think it might be a nice solution to have a security parameter which contained type information about the roles that are allowed access and to implement the schema documentation there. But for now that seems to big of a change for this project. The role information is already there in the middleware and available at runtime, it seems reasonable that middleware could alter the spec as it could change anything about the handling of resources.

I have only worked on this one actix web project so I don't know anything about how they generally look, but I feel that more extensibility would be helpful to paperclip.

platy avatar Nov 27 '20 10:11 platy

There is a different way to pass data from middleware now ReqData so that might be a better way to modify the schema via wrapped types? https://github.com/wafflespeanut/paperclip/issues/300

dunnock avatar Feb 15 '21 11:02 dunnock

@dunnock Thank's for the info - I'll keep that in mind if we update to actix-web 3 at some point

platy avatar Feb 22 '21 08:02 platy