Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to specify request type in optional patterns #64

Open
hiscaler opened this issue Feb 26, 2023 · 14 comments
Open

Add ability to specify request type in optional patterns #64

hiscaler opened this issue Feb 26, 2023 · 14 comments
Labels
status:ready for adoption Feel free to implement this issue.

Comments

@hiscaler
Copy link

 Group::create("/products")
        ->withCors(Cors::class)
        ->routes()
        ->routes(
            Route::get('')
                ->action([ProductController::class, 'index'])
                ->name('product/index'),
            Route::get('/{id:\d+}')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'view'])
                ->name('product/view'),
            Route::post('')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'create'])
                ->name('product/create'),
            Route::post('/{id:\d+}')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'update'])
                ->name('product/update'),
            Route::delete('/{id:\d+}')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'delete'])
                ->name('product/delete'),
            Route::put('/{id:\d+}/undo')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'undo'])
                ->name('product/undo'),
            Route::post('/{id:\d+}/favorite')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'favorite'])
                ->name('product/favorite'),
            Route::delete('/{id:\d+}/favorite')
                ->middleware(Authentication::class)
                ->action([ProductController::class, 'cancelFavorite'])
                ->name('product/cancelFavorite'),
        ),

like product/view if user is login will return 'favorite': true, if not login will return 'favorite': false

but if is use Authentication middleware, must login.

is can use follow code

Authentication::class => [
        'class' => Authentication::class,
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(PassportRequestErrorHandler::class),
        ],
        'withOptionalPatterns()' => [
            'optional' => ['/en/products/[1-9]']
        ]
    ],

but delete,update action url is like to view.
what can i do?

@rustamwin
Copy link
Member

  1. Remove Authentication middleware from the route.
  2. Use $currentUser->isGuest() in your controller.

@hiscaler
Copy link
Author

@rustamwin If remove Authentication middleware in route, $currentUser->isGuest() alway return true.

@vjik
Copy link
Member

vjik commented Feb 26, 2023

@hiscaler You should add optional routes to withOptionalPatterns() method. For example:

Authentication::class => [
        'class' => Authentication::class,
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(PassportRequestErrorHandler::class),
        ],
        'withOptionalPatterns()' => [
            'optional' => [
                '/en/products/[1-9]',
                'products/product/view', // Don't forget adapt pattern for your case
            ]
        ]
    ],

@vjik vjik closed this as completed Feb 26, 2023
@hiscaler hiscaler changed the title Can i not throw err if not lgin Can i not throw err if not login Feb 27, 2023
@hiscaler
Copy link
Author

hiscaler commented Feb 27, 2023

@vjik I will try. Thank you!

@hiscaler
Copy link
Author

@vjik Can not.

GET /en/products/1 is return product 1 detail
PUT /en/products/1 is update product 1 detail
DELETE /en/products/1 is delete product 1

but withOptionalPatterns.optional

['/en/products/[1-9]']

can't not know what's request method.

@hiscaler
Copy link
Author

and has a other question.

['/en/products/[1-9]{1}']

can't matched /en/products/123

@rustamwin
Copy link
Member

@hiscaler
Copy link
Author

@rustamwin Thank you!

@hiscaler
Copy link
Author

@hiscaler see patterns https://github.com/yiisoft/strings/blob/master/README.md#wildcardpattern-usage

Valid setting is ['/en/products/[1-9]*'] , thank you.

@hiscaler
Copy link
Author

@vjik Can not.

GET /en/products/1 is return product 1 detail PUT /en/products/1 is update product 1 detail DELETE /en/products/1 is delete product 1

but withOptionalPatterns.optional

['/en/products/[1-9]']

can't not know what's request method.

So i think should add check request method in Authentication.isOptional

@vjik
Copy link
Member

vjik commented Feb 27, 2023

@hiscaler you want make optional authentication for GET request only and otherwise required (POST, DELETE, etc.)?

@hiscaler
Copy link
Author

@vjik yes, is only GET request, POST, DELETE is must authentication

@vjik
Copy link
Member

vjik commented Mar 1, 2023

Need add ability to specify request type in optional patterns.

@vjik vjik reopened this Mar 1, 2023
@vjik vjik changed the title Can i not throw err if not login Add ability to specify request type in optional patterns Mar 1, 2023
@vjik vjik added status:ready for adoption Feel free to implement this issue. and removed question labels Mar 1, 2023
@vjik
Copy link
Member

vjik commented Mar 2, 2023

Another idea: leave in Authentication middleware authentication process only and create new middleawre that will be throw exception or return response "No access".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue.
Projects
None yet
Development

No branches or pull requests

3 participants