jfrag1 commented on code in PR #24301:
URL: https://github.com/apache/superset/pull/24301#discussion_r1232481129
##########
superset/queries/saved_queries/dao.py:
##########
@@ -44,3 +47,20 @@ def bulk_delete(models: Optional[list[SavedQuery]], commit:
bool = True) -> None
except SQLAlchemyError as ex:
db.session.rollback()
raise DAODeleteFailedError() from ex
+
+ @classmethod
+ def get_by_id(cls, _id: str) -> Optional[SavedQuery]:
Review Comment:
I strongly disagree with the idea that there's no justification for UUID's
being treated differently for security.
> Granted it’s harder to forge a UUID as opposed to guess a valid ID number,
but that still doesn’t seem secure.
Guesswork has nothing to do with it; sequential ID's are vulnerable to
enumeration, meaning a user of the API could easily get access to all saved
queries. This is the case with the old API with no security filter. Consider a
script like the following:
```python
for i in range(1000):
res = requests.get(f'savedqueryapi/get/{i}')
# save response data somewhere
```
UUID's on the other hand, aren't "harder" to guess, they're _impossible_ to
guess. In the v1 API, the only way to obtain a saved query's UUID is by being
logged in as the user who created the saved query, so the contents of the saved
query are as secure as the user keeps the UUID/link to it. I'd argue that this
was the intent that saved queries were created with. It's the way they've
worked until [this
change](https://github.com/apache/superset/pull/21682/files#diff-1fe9f23910bda67f66ff7b5e9127cdaba0d304153a5246dbcb2d76d70d2e5a4fR1365)
was made. There's a "Copy link" button prominently in the UI, which seems to
suggest that the idea is that you can then send that link to someone else to
view.
While yes, the old API which enabled this functionality was not secure
because it allowed enumeration, it was still a great feature to use. I believe
this PR brings the saved query feature back to what it was originally supposed
to be in a much more secure way than before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
