Skip to content

bug: api key not refreshed when api key expired but oauth2 access token not #17070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
1 task done
hugodutka opened this issue Mar 24, 2025 · 4 comments · Fixed by #17878
Closed
1 task done

bug: api key not refreshed when api key expired but oauth2 access token not #17070

hugodutka opened this issue Mar 24, 2025 · 4 comments · Fixed by #17878
Assignees
@spikecurtis
Labels
s2 Broken use cases or features (with a workaround). Only humans may set this.

Comments

@hugodutka
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

We have a system that refreshes expired api keys backed by oauth2 if a refresh token is available.

However, in the case when the api key is expired but the oauth2 access token isn’t, the refresh logic is not triggered, and the api key ExpiresAt field is not updated.

Relavant check:

coder/coderd/httpmw/apikey.go

Line 261 in bbe7dac

if link.OAuthExpiry.Before(now) && !link.OAuthExpiry.IsZero() && link.OAuthRefreshToken != "" {

Early exit if ExpiresAt is not updated.

coder/coderd/httpmw/apikey.go

Lines 319 to 324 in bbe7dac

if key.ExpiresAt.Before(now) {
return optionalWrite(http.StatusUnauthorized, codersdk.Response{
Message: SignedOutErrorMessage,
Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
})
}

Relevant Log Output



Expected Behavior


I'd expect the ExpiresAt field to be updated when a valid OAuth2 access token is available.


Steps to Reproduce


The way I triggered it is I manually updated the ExpiresAt field on an api key to the current time via a SQL query and then refreshed the Coder web UI.


Environment


    

  • Host OS: Linux
    • 

  • Coder version: 2.20.2
    • 



Additional Context


No response

      		
    

      
  





        



            
            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @hugodutka
hugodutka




          added
  the 

  needs-triage

  Issue that require triage
 label


      Mar 24, 2025

    

  








      

  
      



  

  @bpmct





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  


  

    

      

      
            bpmct
  

      

      

      commented


        Mar 24, 2025


      
    


  





      


        


  
    

      
          
@coder

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
      



  

  @github-actions

    
      GitHub Actions
    




  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  


  

    

  





      


        


  
    

      
          
Starting a Coder workspace. You can track the progress here.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
      



  

  @github-actions

    
      GitHub Actions
    




  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  


  

    

  





      


        


  
    

      
          
✅ Workspace has been successfully started! You can access it here.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
            

    

      
    

    


      

          @matifali
matifali




            added
      

  s2

  Broken use cases or features (with a workaround). Only humans may set this.

    and removed
      

  needs-triage

  Issue that require triage

  labels


      May 7, 2025

    

  



    

    

      
    

    


      

          @f0ssel
f0ssel




          assigned     f0ssel and     spikecurtis   and unassigned     f0ssel

      May 8, 2025

    

  



  


  
    
  
  

    
  ibetitsmike 

      added a commit
      that referenced
      this issue

    
      May 13, 2025
    
    

      


  

    

      


  

      
        @ibetitsmike
  





      

        
          fix: update API key expiration based on OAuth refresh
        

          
            
          

      


      

          

    
    
    

  

      


      

          

    
    
    

  

  

    
  



      


      
      

        
          abdddb8
        
      

    

  

    

      
This fixes issue #17070 by refreshing an expired API key when a valid
OAuth2 token is available. Previously, the logic would only refresh the
OAuth token when it was expired, not when just the API key was expired.

The issue occurs because the API key expiration check happened before the
OAuth refresh logic could update the key.ExpiresAt field. Now we attempt
to refresh the OAuth token even when only the API key is expired.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

    




    

  









      

  
      



  

  @spikecurtis





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
So, we have 2 interrelated systems of token lifetimes: The APIKey and the OAuth. I think the actual bug here is intermingling the properties of these two systems.


The OAuth token includes an expiration and a refresh. If the token expires, we try the refresh. If successful, then the user is still authenticated to their OAuth identity. In this sense, the exceeding the expiry is "conditional" --- the IDP might allow us to refresh or not.


The APIKeys use an expiry and a lifetime. We set the lifetime at the time we mint the token, which also determines the initial expiry. But, if we log in with the APIKey, we re-extend the expiry to be now + lifetime. Exceeding the expiry is not conditional. If you don't log in with it at some point during its lifetime it is no longer valid.


At least, that's how APIKeys work in isolation. In actual fact, if the APIKey is backed by OAuth, then when we refresh the OAuth token (but only on refresh, not on initial minting of the APIKey from OAuth) we also extend the the APIKey expiry, BUT! only if the new expiration is longer than the lifetime. Confused? It's confusing!


In addition to being confusing, you can get into this nonsensical state where if you log in now your APIKey will get rejected, but if you waited longer until the OAuth token expires, you'd succeed!


However, I disagree with the "Expected Behavior" of this issue's author:


    

  • we should stop modifying the APIKey's expiry (or any other property) based on OAuth. If the APIKey is expired, it's expired.
    • 

  • any APIKey tied to OAuth should continue to check the OAuth token validity; if either the APIKey or the OAuth token is expired we fail login.
    • 



I think it helps to consider which entities are demonstrating possession of which tokens and what they mean. Consider a hypothetical user "Alice." Coder server possesses Alice's OAuth token, and it means that the Alice identity we obtained from the IDP is still good. The API requestor has demonstrated possession of the APIKey (not the OAuth token). If the APIKey is valid, it means we accept them as authentically representing Alice. If the APIKey is invalid, we don't accept them as authentically representing Alice, even if the Alice identity is still a valid identity from the IDP. If the OAuth token is expired and can't be refreshed, then we can't let them log in, even if we believe they authentically represent Alice, because the Alice identity itself is no longer valid.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
          

  

    
  


  

      

    @spikecurtis
spikecurtis



    mentioned this issue
    
      May 16, 2025
    





  


  

      
   Merged

  







  




    

    


  


  
    
  
  

    
  spikecurtis 

      added a commit
      that referenced
      this issue

    
      May 19, 2025
    
    

      


  

    

      


  

      
        @spikecurtis
  





      

        
          fix: stop extending API key access if OIDC refresh is available (#17878)
        

          
            
          

      


      

          

    
    
    

  

    


      


      

          

    
    
    

  

  

    
  



      


      
      

        
          1a41608
        
      

    

  

    

      
fixes #17070

Cleans up our handling of APIKey expiration and OIDC to keep them separate concepts. For an OIDC-login APIKey, both the APIKey and OIDC link must be valid to login. If the OIDC link is expired and we have a refresh token, we will attempt to refresh.

OIDC refreshes do not have any effect on APIKey expiry.

#17070 (comment) explains why this is the correct behavior.

    




    

  













  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

      

        

  
      @spikecurtis
  
    spikecurtis
    

      








      


  


  

    Labels
  



    

      

    s2

  Broken use cases or features (with a workaround). Only humans may set this.








      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    

      
        

          
    

    Development
  




            
  
Successfully merging a pull request may close this issue.





    


  
          
      fix: stop extending API key access if OIDC refresh is available

    
      coder/coder
    





      
    

  




      

    

  
      

  

    

      5 participants
    

    

        
          @spikecurtis 
        
          @matifali 
        
          @f0ssel 
        
          @bpmct 
        
          @hugodutka