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

Bug: HTTP CREATE not working when using TOKEN auth #4058

Closed
2 tasks done
gander opened this issue May 17, 2024 · 10 comments
Closed
2 tasks done

Bug: HTTP CREATE not working when using TOKEN auth #4058

gander opened this issue May 17, 2024 · 10 comments
Assignees
Labels
feature New feature or request topic:security This is related to security

Comments

@gander
Copy link
Contributor

gander commented May 17, 2024

Describe the bug

Even though I successfully complete the token authorization and send a request to create a user, the record is not created.

Steps to reproduce

Follow instructions of:

Working example: https://surrealdb-auth0-examples.pages.dev/

Expected behaviour

Create and return user record.

SurrealDB version

1.5.0 for linux amd64

Contact Details

adam.gasowski@gander.pl

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • I agree to follow this project's Code of Conduct
@gander gander added bug Something isn't working triage This issue is new labels May 17, 2024
@gguillemas gguillemas self-assigned this May 17, 2024
@gguillemas gguillemas added topic:security This is related to security and removed triage This issue is new labels May 17, 2024
@gguillemas
Copy link
Contributor

Hi @gander! Thank you for reporting this issue as well as surrealdb/docs.surrealdb.com#543, which I will address separately. Specially, thank you for providing a sandbox to help with reproducing this issue.

Can you share the output of INFO FOR DATABASE and INFO FOR SCOPE? It seems like the permissions clauses may not be matching the claims in your token, so both the SELECT and CREATE queries are returning empty. If you try and manually create a user in SurrealDB, does the SELECT query still return empty? If so, I would say this is most likely a problem with the permissions clauses failing to evaluate to true.

@gguillemas
Copy link
Contributor

gguillemas commented May 17, 2024

Looking at the code, I may have found the reason for your issue. All claims that are expected by SurrealDB accept the namespace prefix and are mapped to their original name, so https://surrealdb.com/ns becomes accessible through $token.ns. However, custom claims are extracted from the token as is, so https://surrealdb.com/custom_claim would become accessible through $token.https://surrealdb.com/custom_claim, which I am not sure would even parse in SurrealQL. You may try to access the custom claims in this way in your permissions check to work around the the issue, but I think we should strip the namespace prefix as well for custom claims whenever present so they can be accessed more naturally. I will look into this next week 👍

Thank you again for your detailed report!

@gander
Copy link
Contributor Author

gander commented May 17, 2024

@gguillemas This makes the implementation fully working:

DEFINE TABLE user TYPE ANY SCHEMAFULL PERMISSIONS FOR select, create, update, delete WHERE $scope = 'user' AND $token.aud CONTAINS 'https://gander.eu.auth0.com/api/v2/' AND $token.aud CONTAINS 'https://gander.eu.auth0.com/userinfo' AND email = $token['https://surrealdb.com/email'] AND $token['https://surrealdb.com/email_verified'] = true

@gander
Copy link
Contributor Author

gander commented May 21, 2024

@gguillemas you will still use my working example because I wanted to continue working.

@gguillemas
Copy link
Contributor

gguillemas commented May 21, 2024

If you are asking whether or not the provided implementation will work after the fix is merged, the answer would be no, since the prefix will be stripped and you will be able to use the "correct" way of accessing the namespaced claims:

DEFINE TABLE user TYPE ANY SCHEMAFULL PERMISSIONS FOR select, create, update, delete WHERE $scope = 'user' AND $token.aud CONTAINS 'https://gander.eu.auth0.com/api/v2/' AND $token.aud CONTAINS 'https://gander.eu.auth0.com/userinfo' AND email = $token.email AND $token.email_verified = true

On second thought, I will update the PR to also include the claim with the full namespace prefix to keep backward compatibility on 1.X and port the fix without that only to 2.X. In this way, your current code will continue working.

Let me know if I did not correctly understand what you meant.

TL;DR: I will try to keep backward compatibility. You can assume that the code you shared will continue working unless I am unable to make it so. Check on #4061 for updates!

@gguillemas
Copy link
Contributor

gguillemas commented May 21, 2024

I can confirm that I have been able to keep backward compatibility and your code will still work in 1.5.1 when we release the fix. However, I would still recommend accessing the claims by their canonical name (i.e. without the namespace prefix) as that will be the only way to do so in version 2.X.

@gander
Copy link
Contributor Author

gander commented May 21, 2024

@gguillemas No, I wanted to continue working on my application as it works, i.e. with a prefix. I'm wondering if you'll need a preview of this application for anything else? The application is a Proof of Concept and Work in Progress

@gguillemas
Copy link
Contributor

Correct, you can continue relying on your working code, we do not want to break the current behavior until 2.0 👍
I don't think I will need the preview any more, so you can take it down if you want to. Thank you, it was very helpful.

@gguillemas gguillemas added feature New feature or request and removed bug Something isn't working labels May 24, 2024
@gguillemas
Copy link
Contributor

gguillemas commented May 24, 2024

I have changed this from a bug to a feature. The current behavior to access namespaced claims has been documented in surrealdb/docs.surrealdb.com#557. There are certainly improvements that can be made on how SurrealDB deals with namespaced claims, including allowing the user to set the namespace prefix and exposing its "reserved" claims (e.g. ns, db, id) also through their prefixed name. We may also consider exposing them only through their prefixed name to allow their non-namespaced counterparts to be present and accessible in the token, although that would be a breaking change that is not planned for 2.0.

To answer your question again, we plan on the behavior remaining the same (or compatible) for the foreseeable future.

Thank you for opening this issue, it has helped us review how we deal with namespaced claims and identify some improvements!

@gguillemas
Copy link
Contributor

I will close this issue and keep the original surrealdb/docs.surrealdb.com#543 until it is resolved.
Feel free to create a new issue if you are interested in customizing claim namespaces or any other improvements 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request topic:security This is related to security
Projects
None yet
Development

No branches or pull requests

2 participants