Skip to content

DRAFT: API endpoint for user management#2734

Closed
itbane wants to merge 2 commits intoBookStackApp:developmentfrom
itbane:api-endpoint-users
Closed

DRAFT: API endpoint for user management#2734
itbane wants to merge 2 commits intoBookStackApp:developmentfrom
itbane:api-endpoint-users

Conversation

@itbane
Copy link
Copy Markdown
Contributor

@itbane itbane commented May 6, 2021

Hi,

I've found the need to get users (and their email) from the API for automation purposes. As there isn't an endpoint for user management yet, I thought I'd give it a shot and tried to implement it.

I'd like to get some feedback on whether I'm on the right track and should continue working on this.

Disclaimer: I haven't worked with Laravel ever before and my last contact with php was ~10 years ago. I'm grateful for any advice and pointers you can give me to improve the PR.

Tests and CREATE, UPDATE, DELETE operations are still WIP. I've seen someone request SCIM-Support (#2701) - I'd have to look into that.

Current state:

  • GET: /api/users - Route (only accessible with users-manage permissions); lists all users + their roles
  • GET: /api/users/{id} - Route (only accessible with users-manage permissions); lists details of a specific user

Quick overview on my changes / thought process:

  • Add new routes for users
  • Add new Controller UserApiController
  • Use as much from existing User and UserRepo-classes as possible
    • obstacle: apiListingResponse requires a Builder-object, User::getAll() returns a collection
      • If I understood correctly, the collection contains all the data already, while the builder only contains the databasequery
      • solution: Implemented a new Function within UserRepo that returns a Builder-object
    • obstacle: some fields in the User-object are hidden, but need to be displayed, such as email
      • solution: added a new optional parameter to apiListingResponse that takes an array of fields that should be returned anyways (using data->makeVisible() in ListingResponseBuilder::to_response)

itbane added 2 commits May 5, 2021 14:16
* add /users/{id} to get a single user
* add variable to print fields that are otherwise hidden (e.g. email)
@itbane itbane marked this pull request as draft May 6, 2021 10:35
@ssddanbrown
Copy link
Copy Markdown
Member

Looks good so far @itbane. The create/update endpoints will be the trickier ones, Lots of conditional logic tied up on those requests. Don't worry about covering every use-case for the initial implementation, simplicity is preferred.

If you haven't already worked it out, Comments on the controller methods are automatically used as the description for the endpoint in the docs. We'll also need to ensure requests & responses exist in the dev/api folder which are also used in the docs. Will also need tests to at least cover the main positive case for each endpoint.

@itbane
Copy link
Copy Markdown
Contributor Author

itbane commented May 7, 2021

Great! It'll take a couple of days, but I'll keep adding endpoints, documentation and tests.

@xinin
Copy link
Copy Markdown

xinin commented Nov 10, 2021

Please, we need those features as soon as posible.
We are trying to use the API to create automatically our users and set their permissions.

It would be awesome to be able to change permissions set of a book, shelves ... using your API

@ssddanbrown
Copy link
Copy Markdown
Member

Hi @itbane, How is this going? No massive rush from my side but if you get to a point where you can't progress, or can't devote additional time for any reason, then that's fine but just let me know and I'll consider this as available to pick up & continue.

ssddanbrown added a commit that referenced this pull request Feb 3, 2022
- Updated routes to use new format.
- Changed how hidden fields are exposed to be more flexible to different
  use-cases.
- Updated properties available on read/list results.
- Started adding testing coverage.
- Removed old unused UserRepo 'getAllUsers' function.

Related to #2701, Progression of #2734
@ssddanbrown ssddanbrown mentioned this pull request Feb 3, 2022
7 tasks
@ssddanbrown
Copy link
Copy Markdown
Member

Closing this in favour of progress being continued in #3238, due to lack of response/progress here.
Thanks @itbane for your original starting efforts here.

@ssddanbrown ssddanbrown closed this Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants