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

Extractor doesn't consider inherited functions to implement routes #28

Open
theCalcaholic opened this issue Nov 6, 2023 · 8 comments
Open
Labels
bug Something isn't working

Comments

@theCalcaholic
Copy link

If a route is configured that is implemented by a controller's parent class, an error is thrown saying 'Missing controller method'.

In my case, the method in question is the method authenticate from OCP\AppFramework\AuthPublicShareController which can't be overwritten by my controller because it is final.

As a workaround, I'll try to a proxy function that forwards its parameters to parent::authenticate, but ideally that should not be necessary.

My relevant routes:

<?php
declare(strict_types=1);

return [
	'routes' => [
		// ....
		['name' => 'SecretShare#authenticate', 'root' => '/secret',
			'url' => '/show/{token}/authenticate/{redirect}', 'verb' => 'POST'],
	],
	'ocs' => [
                // ....
	]
];

Excerpt from my controller class:

<?php
declare(strict_types=1);

namespace OCA\Secrets\Controller;

use OCP\AppFramework\AuthPublicShareController;

class SecretShareController extends AuthPublicShareController {
	// ...
}
@provokateurin
Copy link
Member

This problem comes from the fact that we only parse the controller class and no references are resolved. I don't think this is fixable without a huge amount of work, although it already created problems elsewhere, too.

@theCalcaholic
Copy link
Author

@provokateurin A nicer workaround would be to allow ignoring routes from routes.php (as we can't annotate a method that doesn't exist with the #ignoreOpenApi flag).

Maybe something like

return [
    'routes' => [
        [ 
            'name' => 'MyRoute#parentFunc',
            'url' => '/my-url', 
            'ignoreOpenAPI' => true 
        ]
    ]
]

@provokateurin
Copy link
Member

I'm not sure if that is a good solution for this problem. You'd still want to have all the routes, so it wouldn't really make sense to ignore some. Honestly I find it a bit strange to use an inherited controller method, so maybe you can explain your use-case to me?

@theCalcaholic
Copy link
Author

theCalcaholic commented Nov 6, 2023

I'm implementing a password protected share. This uses a controller which inherits from AuthPublicShareController and exposes the authenticate method of it's parent for the authentication result.

I'm following this (admittedly very short) documentation: https://docs.nextcloud.com/server/latest/developer_manual/digging_deeper/publicpage.html#implementing-an-authenticated-public-page

@theCalcaholic
Copy link
Author

theCalcaholic commented Nov 6, 2023

FYI, the workaround mentioned in my original issue doesn't seem to work that easily.

For some reason this redirects to a login page instead of the public share after entering the share password:

SecretShareController.php (method signature and attributes are copied from parent class)

class SecretShareController extends AuthPublicShareController {
// ...
        #[BruteForceProtection(action: 'publicLinkAuth')]
	#[PublicPage]
	#[UseSession]
	public function doAuth(string $password = '', string $passwordRequest = 'no', string $identityToken = '') {
		return parent::authenticate($password, $passwordRequest, $identityToken);
	}
// ...
}

routes.php

return [
    'routes' => [
        [
	    'name' => 'SecretShare#doAuth',
            'url' => '/share/{token}/authenticate/{redirect}',
            'verb' => 'POST'
         ],
    ]
]

So right now I'm stuck with commenting and uncommenting the route during openapi spec generation

@provokateurin
Copy link
Member

That's strange, it should really just work 🤔

@provokateurin
Copy link
Member

So psalm has some explains on how they resolve all the classes they need: https://github.com/vimeo/psalm/blob/master/docs/contributing/how_psalm_works.md
We'd probably need to replicate most of the code in https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php

@provokateurin
Copy link
Member

Implementing this would also make it possible to use other types from PHP like enums and fake enums.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants