Use index-level instead of realm-level credential caching for known indexes#12717
Conversation
jtfmumm
force-pushed
the
jtfm/index-level-credential-cache
branch
2 times, most recently
from
89e68d6 to
aade9e3
Compare
|
@zanieb We can either merge this into jtfm/index-url-auth if approved or sequence it after #12651. |
| let credentials = if let Some(index_url) = maybe_index_url { | ||
| self.cache().get_url(index_url, &username) | ||
| } else { | ||
| // Since there is no known index for this URL, check if there are credentials in | ||
| // the realm-level cache. | ||
| self.cache() | ||
| .get_realm(Realm::from(retry_request.url()), username) | ||
| } | ||
| .or(credentials); | ||
|
|
There was a problem hiding this comment.
If there's no cache entry for the index, should we fall back to the realm still? I would think so, for cases where a file is hosted elsewhere on the realm?
There was a problem hiding this comment.
The problem is that two indexes could have different credentials but the same realm. If both indexes have files hosted elsewhere on the realm, we could only cache the correct credentials at the realm level for one of them. That seems confusing.
But if we did want to fall back to the realm, we couldn't do it here. Instead, we'd need to check the realm cache again after the credentials fetch step. Otherwise uv will potentially find the wrong credentials here and use those without, e.g., checking the keyring for the index URL. There's a test in this PR that covers that case.
There was a problem hiding this comment.
The problem is that two indexes could have different credentials but the same realm. If both indexes have files hosted elsewhere on the realm, we could only cache the correct credentials at the realm level for one of them. That seems confusing.
I agree it seems confusing, but it's also better than not authenticating at all, right? I think the more common case is that you have a single index per realm — mixing indexes per realm is rare. As-is, this could be a regression from our existing behavior.
Otherwise uv will potentially find the wrong credentials here and use those without, e.g., checking the keyring for the index URL.
I agree we shouldn't use the realm-level cache until we've checked for credentials at the index-level.
There was a problem hiding this comment.
Ok, I've added realm-level cache check as a final fallback for an index URL (and a new test to check this behavior)
jtfmumm
force-pushed
the
jtfm/index-level-credential-cache
branch
3 times, most recently
from
a7eef8e to
37becc5
Compare
jtfmumm
force-pushed
the
jtfm/index-level-credential-cache
branch
from
37becc5 to
d89e9a8
Compare
…ndexes (#12717) The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior. Depends on #12651.
…ndexes (#12717) The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior. Depends on #12651.
…ndexes (#12717) The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior. Depends on #12651.
…ndexes (#12717) The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior. Depends on #12651.
2 participants
The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior.
Depends on #12651.