Codecov Report

Merging #465 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #465   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines        2225   2230    +5     
  Branches      437    437           
=====================================
+ Hits         2225   2230    +5

I don't know if this is a good idea or not. I think @samuelcolvin would prefer not to do something like this.

@samuelcolvin

The reason I made changes to the str function for both is that if we keep it as is, then we end up with these kinds of things as JSON output:

>>> import pydantic
>>> class Settings(pydantic.BaseModel):
...     passw: pydantic.SecretStr
...
>>> conf = Settings(passw="1234")
>>> conf
<Settings passw=SecretStr('**********')>
>>> conf.dict()
{'passw': SecretStr('**********')}
>>> conf.json()
'{"passw": "SecretStr(\'**********\')"}'

Where as with the modified str we would get something like this instead:

>>> import pydantic
>>> class Settings(pydantic.BaseModel):
...     passw: pydantic.SecretStr
...
>>> conf = Settings(passw="1234")
>>> conf
<Settings passw=SecretStr('**********')>
>>> conf.dict()
{'passw': SecretStr('**********')}
>>> conf.json()
'{"passw": "**********"}'

Please let me know what you think.

Ok, let's add a display() method to both with returns either None or ********** and use that in JSON, then keep str and repr the same.

That will then need one line the docs/example to explain/show it.

@samuelcolvin is that something like what you were thinking?

Yes exactly.

@samuelcolvin I've been thinking about how to deal with a situation such as this:

from datetime import datetime
from typing import List, Any, Union, Dict
from pydantic import BaseSettings, BaseModel, SecretStr, validator
import os

class Test:
    def __init__(self, config):
        self.config = config
        self.uri = f"http://{config['USERNAME']}:{config['PASSWORD']}@{config['HOST']}"  # Won't work!
    

class NestedSettings(BaseModel):
    HOST: str
    PASSWORD: SecretStr
    USERNAME: SecretStr
    

class Settings(BaseModel):
    USERNAME: SecretStr = SecretStr("")
    PASSWORD: SecretStr = SecretStr("")
    HOST: str = "postgres.github.io"
    PORT: int = 5432
    DB_NAME: str = "testdb"
    VARS: NestedSettings
    
    DB_URI: SecretStr = SecretStr("")
    
    
    @validator("DB_URI", pre=True, whole=True, always=True)
    def set_db_uri(cls, v, values, **kwargs):
        """Set DB_URI."""
        username = values["USERNAME"].get_secret_value()
        password = values["PASSWORD"].get_secret_value()
        host = values["HOST"]
        port = values["PORT"]
        db_name = values["DB_NAME"]
        result = f"postgresql://{username}:{password}@{host}:{port}/{db_name}"
        return result
        
    
def main():
    external_data = {"USERNAME": "abc", "PASSWORD": "def", "VARS": {"HOST": "www.google.com", "USERNAME": "user1", "PASSWORD":"passw"}}
    conf = Settings(**external_data)
    print(conf.dict())
    testobj = Test(conf.dict()["VARS"])
    print(testobj.uri)
    
if __name__ == '__main__':
    main()

Which would result in:

{'VARS': {'HOST': 'www.google.com', 'PASSWORD': SecretStr('**********'), 'USERNAME': SecretStr('**********')}, 'USERNAME': SecretStr('**********'), 'PASSWORD': SecretStr('**********'), 'HOST': 'postgres.github.io', 'PORT': 5432, 'DB_NAME': 'testdb', 'DB_URI': SecretStr('**********')}
http://SecretStr('**********'):SecretStr('**********')@www.google.com

If you need the inner 'object' to be provided for the configuration of another item, you have to kind of force it to fit by modifying the values in-place to be what you want. Like for instance in the above example, where you'd have to unsecret the secret items(manually turn everything into str) before you can proceed.

Which is not especially useful, ideally we would be able to unsecret the entire dict recursively and then pass it on to classes or functions that need those items to be plain-text.

Do you have any thoughts on this?

See dangerouslySetInnerHTML in react.

See hazmat in cryptography.

The whole point is to make it somewhat involved to get the raw value.

If there's any point whatsoever in include SecretStr in pydantic, then we have to make accessing the raw value only possible with get_secret_value() or an internal value.

If people don't need it to be secret, they don't have to use it.

In fact in your example above, it's still not exactly complicated to generate the uri:

f'postgresql://{username.get_secret_value()}:{password.get_secret_value()}@{host}:{port}/{db_name}'

@samuelcolvin In the postgresql example, that would expose secrets if the conf object is printed out.
I think I just need to adjust how I would be using these and use the raw values ocne the config has been loaded, in places where those values will not be exposed.

Anyway, that's not really related to this PR, just a couple of thoughts I was having.
Let me know if you want me to make further changes.

that would expose secrets if the conf object is printed out.

Yes, obviously!

If you use the secret values then they may be visible in the context in which you use them, there's no way around that.

The only point in SecretStr is to avoid it be printed in exceptions or logging (facebook!) until they're used.