Skip to content

Conversation

@Sherpard
Copy link
Member

@Sherpard Sherpard commented Sep 4, 2017

Smart Annotation that takes care of adding a permission check,
User is able to configure how to resolve the CRUD verb implementing CRUDActionResolver on their code

@adrienlauer
Copy link
Member

Can you show a simple example of how to use this annotation ? I'm somewhat understanding its purpose from the test, but an explanation would help :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 68.851% when pulling dfe3da0 on Sherpard:feature/RequiresRestAnnotation into 895cc7b on seedstack:master.

@Sherpard Sherpard changed the title Added @RequiresRest annotation to spec Adds @RequiresCRUD security annotation && Interceptors Sep 5, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 68.584% when pulling 6aa2335 on Sherpard:feature/RequiresRestAnnotation into c9ade71 on seedstack:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 68.584% when pulling 6aa2335 on Sherpard:feature/RequiresRestAnnotation into c9ade71 on seedstack:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 68.662% when pulling cd74b90 on Sherpard:feature/RequiresRestAnnotation into c9ade71 on seedstack:master.

import org.seedstack.seed.security.spi.CRUDActionResolver;

public class TestActionResolver implements CRUDActionResolver {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd name the Class CrudActionResolver in order to be compliant to java standard name with acronyms and also to be flat with CrudSecurityIT.
I also would rename CRUDAction to CrudAction for the same reasons.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I suggested those names but it's better to keep CamelCase even for acryonyms.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, already commited the cangue :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 68.662% when pulling d63d46b on Sherpard:feature/RequiresRestAnnotation into 376894d on seedstack:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 68.646% when pulling e4d3433 on Sherpard:feature/RequiresRestAnnotation into 376894d on seedstack:master.

@adrienlauer
Copy link
Member

The canResolve method is not useful because resolve already returns an Optional and can return empty if nothing is detected. I removed it and refactored the implementation of the interceptors a bit in my fork at https://github.com/adrienlauer/seed/tree/feature/RequiresRestAnnotation.

Added AbstractInterceptor as a base class for the interceptors, as much of their logic were pretty much the same Added SPI functionality to define custom verb resolvers Implementation improvements on CRUD security Simplify CrudActionResolver
@Sherpard Sherpard force-pushed the feature/RequiresRestAnnotation branch from e4d3433 to 6d6464c Compare September 15, 2017 10:00
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 68.741% when pulling d0f6845 on Sherpard:feature/RequiresRestAnnotation into 376894d on seedstack:master.

@adrienlauer adrienlauer merged commit ca2a515 into seedstack:master Sep 15, 2017
@Sherpard Sherpard deleted the feature/RequiresRestAnnotation branch December 16, 2019 10:54
hervestern pushed a commit to hervestern/seed that referenced this pull request Aug 10, 2020
Added AbstractInterceptor as a base class for the interceptors, as much of their logic were pretty much the same Added SPI functionality to define custom verb resolvers Implementation improvements on CRUD security Simplify CrudActionResolver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants