tronikos / opower

A Python library for getting historical and forecasted usage/cost from utilities that use opower.com such as PG&E
Apache License 2.0
65 stars 59 forks source link

Sanitize TOTP secret to prevent binascii.Error: Non-base32 digit found #51

Closed rct closed 1 year ago

rct commented 1 year ago

Slight usability problem with TOTP providers like ConEd - if the user enters a TOTP secret with trailing(*) white space, the exception binascii.Error: Non-base32 digit found is raised by the base64 module. For Home Assistant, the user sees Unknown error which can be resolved by trimming out spaces that might arise from copy and paste. However, this isn't obvious to the user and seems like a problem with the integration.

See https://github.com/home-assistant/core/issues/101829#issuecomment-1783144419

(*) I only tested with trailing white space but I suspect leading white space would cause the same problem.

The Opower config flow in the Home Assistant integration should probably do the cleaning and/or have a check for a TOTP secret with invalid characters.

However, the Opower module itself could remove leading/trailing spaces from the secret and/or validate that the secret s a valid base32 string that can be decoded earlier during configuration. (Right now I think the string that can't be decoded isn't detected until a login is attempted.)

Edit: Forgot to mention, ideally if the opower module detects that the TOTP secret can't be decoded, it should throw an error that will result in a more meaningful error message to the end user.

tronikos commented 1 year ago

Fixed with https://github.com/tronikos/opower/commit/94723fa229c46df666d5ccd790e28712c9b4adc9