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

API endpoint to return User details for a single user #211

Closed
proffalken opened this issue Sep 4, 2023 · 8 comments
Closed

API endpoint to return User details for a single user #211

proffalken opened this issue Sep 4, 2023 · 8 comments

Comments

@proffalken
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The API's for the memberbucks system require the RFID Tag for a user rather than their username.

There are other areas within the platform where it would be good to know this data as well.

At present, the only way to get this data is to call api/admin/members which returns a list of all members, and then iterate over that dict to retrieve the data for the member you require.

Describe the solution you'd like

Update /api/admin/members/<memberid:int>/profile to provide the profile details for a single member based on a GET request

Describe alternatives you've considered
Update the access code to use usernames rather than RFID ID's, but that sounds like a lot of work and solving the wrong problem.

@jabelone
Copy link
Member

jabelone commented Oct 5, 2023

Heya, sorry I've only just been able to get back to this! I'd support adding a GET handler to the existing MemberProfile class in memberportal/api_admin_tools/views.py. That would allow the frontend to get a specific member's profile based on their member ID rather than going through the full list.

Thanks for putting together #212, but I think a better approach would be to implement a more generic "search" endpoint. I'm not a fan of adding use case specific retrieval APIs as they get tricky to maintain over time.

What do you think about updating #212 to instead add support for filter query parameters on the existing endpoint (api/admin/members) you mentioned? (it's the GetMembers class in memberportal/api_admin_tools/views.py) ie if a request included say ?screenName=the_admin, then it could apply a filter to the results it returns. This would be simple to add, backwards compatible, and allow us to add extra filters in the future without creating arbitrary API endpoints. :)

edit: Forgot to explicitly mention, but if you used a filter that should be unique, it will function similarly to a "get member profile with X attribute" like you wanted originally.

@jabelone
Copy link
Member

jabelone commented Oct 5, 2023

The other thing I wanted to mention, is that change you suggested (and mine above) only supports calling that endpoint if you're logged in as an admin account. Is the intention that an external system would be using this endpoint? In that case, it might be better to add a completely new search endpoint like say /api/members set to allow all users (like BumpDoor class in memberportal/api_access/views.py) and handle authentication by checking for an API key.

@proffalken
Copy link
Contributor Author

Happy to adopt that approach, I'll see if I can get something up and running in the next few days and push those changes to #212

@proffalken
Copy link
Contributor Author

@jabelone - looking at this, would you rather this endpoint was under api_general, api_member_tools, or something else completely?

I think the "generic search" endpoint is the best approach here FWIW - I want to have tools like Octoprint or similar be able to get data about an individual based on their username so they can "charge" memberbucks based on filament used etc, but I don't want to give them an "admin" account, so this feels sensible.

@jabelone
Copy link
Member

jabelone commented Oct 5, 2023

I reckon let's just add the filtering feature to the existing GetMembers class in memberportal/api_admin_tools/views.py. Then we can add a second permissions class for API keys once I add them (see below).

I've wanted to add support for proper API keys for external systems for a while now, and this is an excellent motivator for it. This looks simple and means whatever endpoints we add the permission class to would be able to use one of those API keys to authenticate instead of an admin account.

@proffalken
Copy link
Contributor Author

proffalken commented Oct 5, 2023

ok, happy to take that approach.

Any particular reason to go down the API Keys route rather than something like OAuth?

It might add a layer of complexity to the client, but it's more of an "industry standard" these days so might make it easier to integrate with other systems in future

Edit: Ignore the above, let's talk about that on a different issue, I'll add the change to api_admin_tools

@proffalken
Copy link
Contributor Author

@jabelone code pushed, I've tested it locally, seems to work as expected :)

@jabelone
Copy link
Member

jabelone commented Oct 7, 2023

Amazing, thanks! I added OAuth 2.0 support a few weeks ago so you can use a Member Matters account to login to any third party service that supports OAuth 2.0 (like the BMS wiki, forum and moodle elearn website). This is great for authenticating users with external systems, but is overkill/too complex a lot of the time for authenticating server to server communications. OAuth is certainly one of the industry standards, but so is API key based authentication which is much easier/simpler to implement so that's why I'd prefer that. :)

Merged.

@jabelone jabelone closed this as completed Oct 7, 2023
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

No branches or pull requests

2 participants