codeant-ai-for-open-source[bot] commented on code in PR #37398:
URL: https://github.com/apache/superset/pull/37398#discussion_r2748468160
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -198,6 +248,28 @@ def impersonate_user(
return url, engine_kwargs
+ @classmethod
+ def get_table_names(
+ cls,
+ database: Database,
+ inspector: Inspector,
+ schema: str | None,
+ ) -> set[str]:
+ """
+ Get all sheets added to the connection.
+
+ For OAuth2 connections, force the OAuth2 dance in case the user
+ doesn't have a token yet to avoid showing table names berofe auth.
+ """
+ if database.is_oauth2_enabled() and not get_oauth2_access_token(
+ database.get_oauth2_config(),
+ database.id,
+ g.user.id,
+ database.db_engine_spec,
Review Comment:
**Suggestion:** Accessing `g.user.id` in `get_table_names` assumes that
`g.user` is always present, which can raise an `AttributeError` (or a Flask
`RuntimeError` if the user is not attached to `g`) when this method is called
in contexts without an authenticated user; defensively retrieving the user ID
before calling `get_oauth2_access_token` avoids this failure while preserving
the intended behavior when a user is available. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Table-listing can error outside authenticated requests.
- ⚠️ Datasource creation UI may fail listing tables.
- ⚠️ Background reflection tasks may crash unexpectedly.
```
</details>
```suggestion
user = getattr(g, "user", None)
user_id = getattr(user, "id", None)
if (
user_id is not None
and database.is_oauth2_enabled()
and not get_oauth2_access_token(
database.get_oauth2_config(),
database.id,
user_id,
database.db_engine_spec,
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the source: see GSheetsEngineSpec.get_table_names at
superset/db_engine_specs/gsheets.py:252-271. The code directly accesses
g.user.id at lines
264-268.
2. Create or simulate a context without a Flask request user (no g.user).
For example,
call GSheetsEngineSpec.get_table_names(database, inspector, None) from a
background task
or unit test where flask.g has no .user attribute (this is a realistic
execution context
because reflection/table listing may be invoked outside a request). The code
path invoked
is the classmethod defined at superset/db_engine_specs/gsheets.py:252.
3. Execution reaches the conditional at
superset/db_engine_specs/gsheets.py:264 which
evaluates get_oauth2_access_token(..., g.user.id, ...). Because g.user is
missing,
accessing g.user.id raises AttributeError (or Flask RuntimeError if g is not
set), causing
the call to short-circuit and raise before get_oauth2_access_token runs.
4. Observe an unhandled AttributeError/RTE originating from
superset/db_engine_specs/gsheets.py:264-268, preventing get_table_names from
returning and
breaking any caller that lists tables (for example, UI table-listing or
automated
reflection tasks). The improved code defends by reading user id with
getattr(g, "user",
None) and only invoking get_oauth2_access_token when a user_id exists,
avoiding the
AttributeError.
5. Note: get_oauth2_access_token is defined in superset/utils/oauth2.py
(matched by
project Grep); protecting g.user usage is appropriate because OAuth2 token
lookup is
per-user and should not be attempted when no user context exists.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/gsheets.py
**Line:** 264:268
**Comment:**
*Possible Bug: Accessing `g.user.id` in `get_table_names` assumes that
`g.user` is always present, which can raise an `AttributeError` (or a Flask
`RuntimeError` if the user is not attached to `g`) when this method is called
in contexts without an authenticated user; defensively retrieving the user ID
before calling `get_oauth2_access_token` avoids this failure while preserving
the intended behavior when a user is available.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
