fix: Make g.user attribute access safe for public users #14287
Conversation
| @@ -1024,7 +1024,7 @@ def estimate_query_cost( | |||
| if not cls.get_allow_cost_estimate(extra): | |||
| raise Exception("Database does not support cost estimation") | |||
|
|
|||
| user_name = g.user.username if g.user else None | |||
| user_name = g.user.username if g.user and hasattr(g.user, "username") else None | |||
There was a problem hiding this comment.
it probably makes more sense to just check if the user is authenticated, using g.user.is_authenticated
There was a problem hiding this comment.
I originally was checking g.user.is_anonymous in these use cases, but opted for the explicit check for username, as this is the value that is being fetched. There may be use cases where username is not present in other states down the road, so thought explicit hasattr would be more future-proof.
| @@ -954,7 +954,7 @@ def favorite_status(self, **kwargs: Any) -> Response: | |||
| charts = ChartDAO.find_by_ids(requested_ids) | |||
| if not charts: | |||
| return self.response_404() | |||
| favorited_chart_ids = ChartDAO.favorited_ids(charts, g.user.id) | |||
| favorited_chart_ids = ChartDAO.favorited_ids(charts, g.user.get_id()) | |||
There was a problem hiding this comment.
|
/testenv up |
|
@robdiciuccio Ephemeral environment spinning up at http://54.201.39.228:8080. Credentials are |
|
Ephemeral environment shutdown and build artifacts deleted. |
mistercrunch
added
🏷️ bot
SUMMARY
Make g.user attribute access safe for public users by:
get_idmethodg.user.usernamebefore using (this property is not set for public users)TEST PLAN
Automated tests should pass.
ADDITIONAL INFORMATION