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

+nodeinfo public service #16231

Merged
merged 2 commits into from
Sep 4, 2019
Merged

+nodeinfo public service #16231

merged 2 commits into from
Sep 4, 2019

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Jul 4, 2019

needed by the social app.

NodeInfo is an effort to create a standardized way of exposing metadata about a server running one of the distributed social networks. The two key goals are being able to get better insights into the user base of distributed social networking and the ability to build tools that allow users to choose the best fitting software and server for their needs.

source: https://github.com/jhass/nodeinfo

@ArtificialOwl
Copy link
Member Author

/backport to stable16

@ArtificialOwl
Copy link
Member Author

/backport to stable15

@MorrisJobke
Copy link
Member

@daita Mind to add a setup check as well and add it to the Nginx config in the docs? The setup check should only run if the social app is enabled.

@MorrisJobke MorrisJobke mentioned this pull request Jul 15, 2019
28 tasks
@MorrisJobke
Copy link
Member

@daita Mind to add a setup check as well and add it to the Nginx config in the docs? The setup check should only run if the social app is enabled.

🏓

@MorrisJobke MorrisJobke added the 2. developing Work in progress label Jul 15, 2019
@ArtificialOwl ArtificialOwl force-pushed the enhancement/noid/nodeinfo branch from bbe34c6 to 39a9b68 Compare July 15, 2019 21:46
@ArtificialOwl
Copy link
Member Author

So, there is some check. Also, this should fix #13088.

I think more logical to returns 404 if the public_service is not set, or if the app is not available/installed (instead of 501)

@rullzer
Copy link
Member

rullzer commented Aug 11, 2019

So is this ready?

@ArtificialOwl
Copy link
Member Author

My bad, totally forgot to push.

However, we also need to define what error code should be returned:

  • when no public service as been assigned.
  • when public service as been assigned, the app is still enabled but the linked php file is not readable.
  • when public service as been assigned but the app is not enabled anymore.

Also: #13088 (comment)

@MorrisJobke 🏓

This was referenced Aug 18, 2019
@ArtificialOwl
Copy link
Member Author

@rullzer can you have a look to my last post regarding this issue?

@rullzer
Copy link
Member

rullzer commented Aug 28, 2019

  • when no public service as been assigned.
  • when public service as been assigned, the app is still enabled but the linked php file is not readable.
  • when public service as been assigned but the app is not enabled anymore.

in all those cases a 404 seems appropriate. And do proper logging to the log file.
For the end user it is all a not found error basically.

@rullzer rullzer mentioned this pull request Aug 29, 2019
16 tasks
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl ArtificialOwl force-pushed the enhancement/noid/nodeinfo branch from 57d7893 to 22b1415 Compare August 29, 2019 17:24
@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Aug 29, 2019

#16919 - so I checked the current implementation, and confirm that this PR:

also rebased.

@nickvergessen @ChristophWurst @danxuliu please review before next beta/rc

@rullzer rullzer added the 3. to review Waiting for reviews label Aug 29, 2019
@nickvergessen
Copy link
Member

So the social app is going to set that config?

@ArtificialOwl
Copy link
Member Author

like the webfinger public service, yes

@rullzer rullzer merged commit 560b985 into master Sep 4, 2019
@rullzer rullzer deleted the enhancement/noid/nodeinfo branch September 4, 2019 08:36
@backportbot-nextcloud
Copy link

The backport to stable16 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable15 failed. Please do this backport manually.

@ArtificialOwl
Copy link
Member Author

@rullzer i'll make a light backport of the fix on the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants