dnplus / streamlit-oauth

Simple OAuth Component for Streamlit App
MIT License
111 stars 18 forks source link

Cached generation of PKCE key pairs #44

Open Sky-Da-Silva opened 1 week ago

Sky-Da-Silva commented 1 week ago

I was just having a look around and noticed something that I think should be changed. Currently, the function _generate_pkce_pair is cached with @st.cache_data - this means that when _generate_pkce_pair(pkce) is called, it won't generate a new key as the function's result will be cached.

@st.cache_data(ttl=300)
def _generate_pkce_pair(pkce):
  """
  generate code_verifier and code_challenge for PKCE
  """
  if pkce != "S256":
    raise Exception("Only S256 is supported")
  code_verifier = secrets.token_urlsafe(96)
  code_challenge = base64.urlsafe_b64encode(hashlib.sha256(code_verifier.encode()).digest()).decode().replace("=", "")
  return code_verifier, code_challenge

This behavior is problematic because the code_verifier and code_challenge should be unique for each authorization request to ensure security. Caching this function causes the same values to be reused, which is not desired.

I think the line @st.cache_data(ttl=300) needs to be removed. I'm new to streamlit so I might be misunderstanding!

dnplus commented 3 days ago

Consider implementing.

Using cache is sharing state across different redirect streamlit session.

Because we use a pop-up window and watch the redirect from parent, the parent window will not get refreshed.

So maybe we can try to control the data within st.session_state