Skip to content

Conversation

@tpoliaw
Copy link
Contributor

@tpoliaw tpoliaw commented Jul 30, 2025

Further prep work for #928

@DiamondJoseph DiamondJoseph changed the title WIP extracting the authenticator model changes from #928 chore(types): Add typing to Authenticator (extracted from #928) Jul 31, 2025
@ZohebShaikh
Copy link
Contributor

return Settings()

This is not giving the authentication settings in
settings: Settings = Depends(get_settings),

Because it is creating a new settings object Will this be addressed in this PR ?

@tpoliaw
Copy link
Contributor Author

tpoliaw commented Jul 31, 2025

Because it is creating a new settings object Will this be addressed in this PR ?

It should be caching the returned settings instance so later modifications to it should be returned next time? It should be fixed but probably not in this PR. Was it working without this change?

@ZohebShaikh
Copy link
Contributor

Because it is creating a new settings object Will this be addressed in this PR ?

It should be caching the returned settings instance so later modifications to it should be returned next time? It should be fixed but probably not in this PR. Was it working without this change?

It is the same behavior as on main as well , It was more of a feature request to have authentication settings come in settings object

tiled/tiled/server/app.py

Lines 430 to 437 in 59cd577

for item in [
"allow_anonymous_access",
"secret_keys",
"single_user_api_key",
"access_token_max_age",
"refresh_token_max_age",
"session_max_age",
]:

In this override settings it never adds the settings for authentication.

@tpoliaw tpoliaw force-pushed the authenticator_models branch from 5f7adb1 to f3c0ca3 Compare July 31, 2025 14:45
@tpoliaw tpoliaw force-pushed the authenticator_models branch from be9f4a9 to 3ed3f65 Compare August 5, 2025 10:49
@tpoliaw tpoliaw marked this pull request as ready for review August 5, 2025 13:10
@tpoliaw tpoliaw force-pushed the authenticator_models branch 5 times, most recently from c380dbf to 10d11e5 Compare August 11, 2025 19:14
@danielballan
Copy link
Member

Give us a nudge when this is ready for 🇺🇸 review. :-)

@tpoliaw tpoliaw force-pushed the authenticator_models branch 3 times, most recently from f2f91a7 to 8bc4f7a Compare August 13, 2025 12:45
@tpoliaw
Copy link
Contributor Author

tpoliaw commented Aug 13, 2025

@danielballan I think it's reviewable now I've patched the rebase I mangled.

@tpoliaw tpoliaw changed the title chore(types): Add typing to Authenticator (extracted from #928) chore(types): Add typing to Authenticator Sep 5, 2025
@danielballan
Copy link
Member

danielballan commented Sep 5, 2025

I rebased. The file test_access_control.py was effectively rewritten, and it no longer defined a custom Authenticator class. Thus, no changes to that file are necessary.

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.

5 participants