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

mu-plugin in playground to disable REST API auth #30

Merged
merged 9 commits into from
Sep 11, 2024

Conversation

ashfame
Copy link
Member

@ashfame ashfame commented Sep 10, 2024

This PR effectively solves our "how to auth" with REST API problem since we do this in playground specific to our browser extension and not in general with the backend plugin.

I ended up trying this "disabling auth" approach because with using password approach, that works with application passwords and not just regular user's password. And even with application passwords, there wasn't an easy way to communicate that to frontend once we create and set that up. Options exist for development purposes but not so much for prod, when folks are using this extension out in the wild.

I think this playground specific effect is quite a good option. Thoughts?

@ashfame ashfame self-assigned this Sep 10, 2024
@ashfame ashfame changed the base branch from trunk to local_js_tests_fix September 10, 2024 12:35
@ashfame ashfame changed the base branch from local_js_tests_fix to trunk September 10, 2024 12:35
@ashfame ashfame force-pushed the disable_rest_api_auth_in_playground branch 2 times, most recently from 7a7a13a to b02dd8e Compare September 10, 2024 13:02
@ashfame ashfame force-pushed the disable_rest_api_auth_in_playground branch from b02dd8e to a31c80e Compare September 10, 2024 13:42
@ashfame ashfame requested a review from psrpinto September 10, 2024 15:23
@ashfame ashfame marked this pull request as ready for review September 10, 2024 15:23
@ashfame ashfame requested a review from akirk September 10, 2024 15:23
@psrpinto
Copy link
Member

psrpinto commented Sep 10, 2024

This is a great solution! However, it doesn't seem to be working, I haven't looked into why that is.

I added a temporary button to create a post (we can delete it before merging this). After clicking the button, in the extension console, you can see that the response is 401: Sorry, you are not allowed to create posts as this user.

@ashfame
Copy link
Member Author

ashfame commented Sep 10, 2024

I haven't tested it in this repo (meaning from the frontend in this repo), but a simple curl command to the REST API and it worked. 401 without it, and successful post creation with it. I will do some digging.

@ashfame
Copy link
Member Author

ashfame commented Sep 11, 2024

@psrpinto You can try now & once you remove the test button, mark this as approved, so that I can merge :)

don't mind the failing test, its from test code, which will be gone.

@psrpinto
Copy link
Member

I removed the button and took the liberty of improving a couple of things.

@ashfame
Copy link
Member Author

ashfame commented Sep 11, 2024

@psrpinto Excellent! I see createPost() is still there, intentional?

@ashfame
Copy link
Member Author

ashfame commented Sep 11, 2024

@psrpinto confirmed its intentionally kept there for reference, so merging now.

@ashfame ashfame merged commit 75d5f2f into trunk Sep 11, 2024
3 checks passed
@ashfame ashfame deleted the disable_rest_api_auth_in_playground branch September 11, 2024 14:14
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.

2 participants