~mediagoblin/mediagoblin#33: 
Login error: TypeError in basic auth's use of bcrypt

When attempting to login to a MediaGoblin instance with basic database authentication enabled (running MediaGoblin master), I'm getting an error. Here's the full traceback:

Traceback (most recent call last):                                                                                                                                                                                 
  File "./mediagoblin/app.py", line 344, in __call__                                                     
    return self.call_backend(environ, start_response)                                                    
  File "/usr/local/lib/python3.9/dist-packages/werkzeug/middleware/shared_data.py", line 260, in __call__                                                                                                          
    return self.app(environ, start_response)                                                             
  File "./mediagoblin/app.py", line 278, in call_backend                                                 
    return self._finish_call_backend(request, environ, start_response)                                                                                                                                             
  File "./mediagoblin/app.py", line 320, in _finish_call_backend                                         
    response = controller(request)                                                                       
  File "./mediagoblin/decorators.py", line 366, in wrapper                                               
    return controller(request, *args, **kwargs)                                                          
  File "./mediagoblin/auth/views.py", line 93, in login                                                  
    user = check_login_simple(                                                                           
  File "./mediagoblin/auth/tools.py", line 172, in check_login_simple                                    
    if not auth.check_password(password, user.pw_hash):                                                                                                                                                            
  File "./mediagoblin/auth/__init__.py", line 42, in check_password                                      
    return hook_handle("auth_check_password",                                                            
  File "./mediagoblin/tools/pluginapi.py", line 306, in hook_handle                                      
    result = callable(*args, **kwargs)                                                                   
  File "./mediagoblin/plugins/basic_auth/__init__.py", line 91, in check_password                                                                                                                                  
    return auth_tools.bcrypt_check_password(raw_pass,                                                    
  File "./mediagoblin/plugins/basic_auth/tools.py", line 51, in bcrypt_check_password                                                                                                                              
    randplus_stored_hash = bcrypt.hashpw(stored_hash, rand_salt)                                         
  File "/usr/local/lib/python3.9/dist-packages/bcrypt/__init__.py", line 79, in hashpw                                                                                                                             
    raise TypeError("Strings must be encoded before hashing")                                            
TypeError: Strings must be encoded before hashing                                       

I suspect that this is the changeset where the problem was introduced: https://git.savannah.gnu.org/cgit/mediagoblin.git/commit/?id=fe01dd00fbebbf46f8cab552b89c402124541cab

Maybe there's a missing str-to-byte conversion?

Status
RESOLVED FIXED
Submitter
~witten
Assigned to
No-one
Submitted
2 years ago
Updated
1 year, 10 months ago
Labels
No labels applied.

~witten 2 years ago · edit

I confirmed that rolling back to the Git changeset before bcrypt was introduced makes the login error go away, and I'm able to login successfully again.

~shtrom 2 years ago

I ran into the same problem when working on the tests.

This commit may fix your issue https://git.sr.ht/~shtrom/mediagoblin/commit/9b052dd53dbbe48de6dd7de67f80e95288a2b1eb

~iptrip 2 years ago*

I'm having this problem, but the URL for the commit shtrom linked to returns 404. I wasn't able to find it in shtrom's log either.

Edit: this commit solved the issue for me (thanks shtrom): https://git.sr.ht/~shtrom/mediagoblin/commit/c6419cc68549d6f331a6fe87e2a19b1b17a2cb78

~shtrom 2 years ago

On Mon 05 Dec 2022 at 03:17:30 +0000, ~iptrip wrote:

I'm having this problem, but the URL for the commit shtrom linked to returns 404. I wasn't able to find it in shtrom's log either.

Did you try this one [0]?

Likely on top of that one if you don't have it [1].

[0] https://git.sr.ht/~shtrom/mediagoblin/commit/d928daee63a52b6ff3d762bf6d6aa534fd7d3684 [1] https://git.sr.ht/~shtrom/mediagoblin/commit/fe01dd00fbebbf46f8cab552b89c402124541cab

-- Olivier Mehani shtrom@ssji.net PGP fingerprint: 4435 CF6A 7C8D DD9B E2DE F5F9 F012 A6E2 98C6 6655 Confidentiality cannot be guaranteed on emails sent or received unencrypted.

~iptrip 2 years ago

I actually just replaced my copy of plugins/basic_auth/tools.py with yours, so I believe all three of those commits were covered. I really appreciate it!

Olivier Mehani 2 years ago · edit

On 6 December 2022 4:09:08 am AEDT, ~iptrip outgoing@sr.ht wrote:

I actually just replaced my copy of plugins/basic_auth/tools.py with yours, so I believe all three of those commits were covered. I really appreciate it!

#Cool. Did that fix your issue? If not, can you get me a full stacktrace of the error?

Olivier Mehani shtrom@ssji.net Sent from my mobile, please excuse my brevity.

~peter_horvath 2 years ago

Before I have found this ticket, also I have found (and fixed) the problem. I second, the patch looks correct. [I intentionally do not link my equivalent patch because I want a quick merge].

~iptrip 2 years ago

Cool. Did that fix your issue?

I apologize for not seeing this sooner, but yes, as I mentioned in the edit to my message and as peter_horvath mentioned, it fixed the issue. It sounds like peter_horvath submitted a merge request but (based on the mailing list) it will be several weeks before the maintainers are able to approve any MRs (busy time of year and all).

~peter_horvath 2 years ago*

On the channels I tried to contact them, the mailing list was the the most responsive until now. Afaik the problem is that only 1 dev is available and even he is busy. I have also seen a that Olivier Mehani has many unmerged branches in his forked repo ( https://git.sr.ht/~shtrom/mediagoblin ).

~sturm REPORTED FIXED 1 year, 10 months ago

This fix is now merged into the master branch. Thanks Olivier!

Register here or Log in to comment, or comment via email.