Skip to content

Allow API endpoint access with a predefined token#269

Merged
PVince81 merged 2 commits intomasterfrom
enh/100/api-with-token
Mar 26, 2021
Merged

Allow API endpoint access with a predefined token#269
PVince81 merged 2 commits intomasterfrom
enh/100/api-with-token

Conversation

@PVince81
Copy link
Member

@PVince81 PVince81 commented Jan 19, 2021

Quick and dirty approach for accessing the metrics endpoint without admin credentials, not that user friendly but works.

Fixes #100

Drawbacks:

  • it uses its own token approach (inspired by how the music app does it for Ampache). Making it use the proper token facility requires many more changes to be able to generate tokens with more restrictions (might need core/server changes)
  • only one token for now
  • not configurable in the web UI, need occ

Issues:

  • unsure about the NC-Token header, need to check if there are other places where we use different headers
  • header vs URL param (which could leak more easily)

@PVince81 PVince81 force-pushed the enh/100/api-with-token branch 2 times, most recently from d98386b to 51477cf Compare January 19, 2021 17:35
@PVince81 PVince81 force-pushed the enh/100/api-with-token branch from 51477cf to 68ebff2 Compare February 26, 2021 10:00
@PVince81
Copy link
Member Author

rebased for CI.

if this minimal approach is acceptable and secure enough I can then move forward to write tests for this

thoughts ? @LukasReschke @nickvergessen @kesselb

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the enh/100/api-with-token branch from 68ebff2 to b855a20 Compare March 1, 2021 20:06
@PVince81
Copy link
Member Author

PVince81 commented Mar 1, 2021

I've added unit tests

@PVince81 PVince81 added this to the Nextcloud 22 milestone Mar 1, 2021
@PVince81 PVince81 marked this pull request as ready for review March 1, 2021 20:07
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

I fixed the info text (which said OC-Token instead of NC-Token). But then it works fine 👍

@PVince81 PVince81 merged commit 87bf21e into master Mar 26, 2021
@delete-merged-branch delete-merged-branch bot deleted the enh/100/api-with-token branch March 26, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

User with limited rights dedicated only for monitoring

3 participants