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

App-Dev: how to implement token in app #17596

Closed
Rello opened this issue Oct 18, 2019 · 18 comments
Closed

App-Dev: how to implement token in app #17596

Rello opened this issue Oct 18, 2019 · 18 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement

Comments

@Rello
Copy link
Contributor

Rello commented Oct 18, 2019

Hello NC masters,

I am working on a new app. this is providing a public page. A token will be used to "hide" the content to be shown. I investigated existing apps...

I found $this->shareManager->newShare(); (used in file_sharing ShareAPIController).
This is using IManager to create a token and store it in oc_share table.
This standard API seems pretty usable!

BUT: IManager only knows 2 types "file" and "folder", so I am not sure if this is the correct way to misuse this for an own app, where the share is not linked to files.

The alternative would be to implement an own token-api-logic in my app and store it in an app-specific _share table, but I like the standard API approach

Any advice?
thank you and keep up NC!

@Rello Rello added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Oct 18, 2019
@Rello
Copy link
Contributor Author

Rello commented Oct 18, 2019

@nickvergessen are you a good partner for this request?

@Rello
Copy link
Contributor Author

Rello commented Oct 19, 2019

as sharing is the core part, a more flexible oc_share approach would sound good to me to reuse all logic including automatic expiration & co

@Rello
Copy link
Contributor Author

Rello commented Oct 19, 2019

nextcloud/deck#14

@Rello
Copy link
Contributor Author

Rello commented Oct 19, 2019

@rullzer
I think you are the master of AuthPublicShareController.php
thank you

@rullzer
Copy link
Member

rullzer commented Oct 21, 2019

Implement your own logic. We used to have it all in one place but this became full of hacks.

@nickvergessen
Copy link
Member

Yeah, talk also has the logic in it's own place. Just much simpler and easier to adjust in case it's needed

@Rello
Copy link
Contributor Author

Rello commented Oct 21, 2019

Hello,
Do you mean to ge-do the AuthPublic with all the automated session/logonscreen/expiration again in every app? Quite some effort and security risk I think.

I think more flexibility in the oc_share table for not only the 2 types would be a good start

@nickvergessen
Copy link
Member

I think more flexibility in the oc_share table for not only the 2 types would be a good start

We really had exactly that in the past. But it is just not suitable, which is why when the calendar/contacts backend was redone, it was moved to it's own table.

A "logonscreen" is as easy as:
https://github.com/nextcloud/spreed/blob/master/lib/Controller/PageController.php#L229-L245

When oc_share contains shares from all the apps, we can not clean it up easily anymore. permissions might or might not work in the same way they do for file, *_id files being integers might be the next problem etc. The list is just too long and you would have to put a lot of effort in the sharing thing additionally causing lots of compatibility issues with the existing users of the API.

@rullzer
Copy link
Member

rullzer commented Oct 21, 2019

@Rello note that you can use the AuthPublicShareController etc just fine. It is an abstraction and you can hook in your own logic.

This means you then just have to verify token (and password) if needed.

@Rello
Copy link
Contributor Author

Rello commented Oct 21, 2019

Hello @nickvergessen
thank you for the hint with spreed. I hacked my own version here which works

BUT: for some reason the session-part is the only that that is not working. the password seems not to be stored in the session. Do I need to initialise the session usage somewhere?

thank you

@nickvergessen
Copy link
Member

You need to add the @UseSession annotation to the doc block. Otherwise the session is closed already and can only be read not written

@Rello
Copy link
Contributor Author

Rello commented Oct 21, 2019

sweet. thank you so much.

one more - could not find it in Talk:
the route has a definition for GET
['name' => 'page#indexPublic', 'url' => '/p/{token}', 'verb' => 'GET'],

But the authentication.php password form triggers a POST to the same page and the route can only have one entry.

Talk has the same setup, but I could not figure out how you got it working because it also only offers

'routes' => [
		[
			'name' => 'Page#index',
			'url' => '/',
			'verb' => 'GET',
		],
	],

@nickvergessen
Copy link
Member

Talk is hardcoded in the server, to allow us to use short links without apps/spreed inside:

server/core/routes.php

Lines 96 to 97 in 9237350

['name' => 'pagecontroller#showCall', 'url' => '/call/{token}', 'verb' => 'GET', 'app' => 'spreed'],
['name' => 'pagecontroller#authenticatePassword', 'url' => '/call/{token}', 'verb' => 'POST', 'app' => 'spreed'],

So yeah, just define a POST route too and it should work

@nickvergessen
Copy link
Member

do you use unique method names? That is important I think.
Otherwise maybe you can point to your code so I can look into it a bit better?

@Rello
Copy link
Contributor Author

Rello commented Oct 22, 2019

It took me 1 night to understand your answer;-)
I will test tonight. Your reference helped

@Rello
Copy link
Contributor Author

Rello commented Oct 23, 2019

Thank you @nickvergessen for helping out. Everything is working now

@Rello Rello closed this as completed Oct 23, 2019
@JenHa
Copy link

JenHa commented Feb 14, 2020

Hi,
I got a similar problem. How did you define your route?
In the AuthPublicShareController the route is build within the method getRoute() and returns somethink like mynamespace.AuthSecretShareController.method, but the route couldn't be found. And I don't know how to define it, so it could be found.
Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement
Projects
None yet
Development

No branches or pull requests

4 participants