search

AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
788 stars 194 forks source link

Disable `allow_reuse_address` under some conditions #418

Closed jiasli closed 2 years ago

jiasli commented 2 years ago

acquire_token_interactive has a port parameter.

When port is explicitly specified, like 8400

On non-Windows platforms (Linux, FreeBSD, etc.)

allow_reuse_address should be set to True, in order to avoid TIME_WAIT and SO_LINGER problem (https://stackoverflow.com/a/14388707/2199657).

This is the default behavior of HTTPServer and why allow_reuse_address gets set to True in the first place (https://github.com/python/cpython/commit/18865de7bd91dfcc42f323440aa85e6f36972c72).

On Windows

This also allows port reuse, making multiple MSAL instances be able to listen to the same port (https://stackoverflow.com/a/14388707/2199657, https://github.com/Azure/azure-cli/issues/10578).

On the other hand, Windows doesn't seem to have TIME_WAIT and SO_LINGER problem by default without SO_EXCLUSIVEADDRUSE (https://github.com/tornadoweb/tornado/issues/550).

Therefore, allow_reuse_address should be disabled (https://github.com/Azure/azure-cli/pull/10955).

When ephemeral port 0 is used

allow_reuse_address may cause an in-use port to be returned, causing EADDRINUSE long before the ephemeral port space has been exhausted (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=174087).

Also, since an ephemeral port is chosen, TIME_WAIT and SO_LINGER won't be a problem.

Therefore, allow_reuse_address should be disabled (https://gavv.github.io/articles/ephemeral-port-reuse/).

Related discussions

rayluo commented 2 years ago

The new expected behaviour would be an exception being raised. And then we can and probably should come up with a unit test case for it.

Implementation wise, perhaps we should move this part of logic into the underlying class _AuthCodeHttpServer.

This part is now implemented in #427. The rest of this PR might not be necessary. Closing this PR, for now. If we feel a need to revisit this topic, we can rebase/reopen this PR later.

Thanks again for @jiasli 's research to bring up this valuable topic in the first place!