Skip to content
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.

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

fix:Session cookie not set #85

Closed
wants to merge 1 commit into from
Closed

fix:Session cookie not set #85

wants to merge 1 commit into from

Conversation

0wei
Copy link

@0wei 0wei commented Mar 1, 2024

Not adding a session resulted in the server consistently returning 401

@rburgst
Copy link
Owner

rburgst commented Mar 1, 2024

thanks a lot for taking the time to contribute, could you be so kind to add a unit test so that we wont break this functionality with future updates?

@rburgst
Copy link
Owner

rburgst commented Mar 6, 2024

did you actually try to assign a cookiejar with the client? I am wondering why the authenticator should deal with this directly.
https://stackoverflow.com/questions/34881775/automatic-cookie-handling-with-okhttp-3

@0wei
Copy link
Author

0wei commented Mar 30, 2024

did you actually try to assign a cookiejar with the client? I am wondering why the authenticator should deal with this directly. https://stackoverflow.com/questions/34881775/automatic-cookie-handling-with-okhttp-3

I'm not use the cookiejar,
okhttp version: implementation 'com.squareup.okhttp3:okhttp:4.11.0'

Here are the issues I discovered while tracking the code.

class RetryAndFollowUpInterceptor(private val client: OkHttpClient) : Interceptor {

  @Throws(IOException::class)
  override fun intercept(chain: Interceptor.Chain): Response {
    while (true) {
        // The response contain cookie
        response = realChain.proceed(request)
        val followUp = followUpRequest(response, exchange)
        // If the request does not contain response cookie, the server will consider it a new request
        // That's why we need add the cookie manually
        request = followUp
      }
    }
  }

  private fun followUpRequest(userResponse: Response, exchange: Exchange?): Request? {
    when (responseCode) {
      HTTP_UNAUTHORIZED -> return client.authenticator.authenticate(route, userResponse)
    }
  }

public class DigestAuthenticator implements CachingAuthenticator {
// com.burgstaller.okhttp.digest.DigestAuthenticator#authenticate
    public synchronized Request authenticate(Route route, Response response) throws IOException {
        Request request = this.authenticateWithState(route, response.request(), parameters);
        // add the cookie manually
        return request;
    }
}

This is the server code I tested

from flask import Flask, redirect, send_file,make_response,jsonify
import socket
from flask_httpauth import HTTPBasicAuth
from werkzeug.security import generate_password_hash, check_password_hash
from flask_httpauth import HTTPDigestAuth

app = Flask(__name__)
app.config['SECRET_KEY'] = '324324243rfdaf2342432'
auth = HTTPDigestAuth()

users = {
    "john": "hello",
    "susan": generate_password_hash("bye")
}


@auth.get_password
def get_pw(username):
    if username in users:
        return users.get(username)


@app.route('/redirect')
def redirect_to_file():
    return redirect('/download')

@auth.error_handler
def unauthorized():
    return make_response(jsonify({'error': 'Unauthorized access'}), 401)
    # return make_response(jsonify({'error': 'Unauthorized access'}), 403) # 403 禁止

@app.route('/download')
@auth.login_required
def download_file():
    filename = './1.mp4'  
    return send_file(filename, as_attachment=True)

if __name__ == '__main__':
    hostname = socket.gethostname()
    ip_address = socket.gethostbyname(hostname)
    
    app.run(host="192.168.2.244",port=5000,debug=True) 

@rburgst
Copy link
Owner

rburgst commented Mar 30, 2024

If you are not using a cookie jar then you are doing it wrong. Please consider the stack overflow article I have linked. You cannot expect other libraries doing the work for you. The Authenticator should usually not have to worry about cookie handling. Also this library exists since years and you are the first one to need that.
I am quite convinced that adding cookie jar to your solution should fix it.

@rburgst rburgst closed this Jul 24, 2024
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.

2 participants