Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Password Hashing update #749

Merged
merged 5 commits into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ The types of changes are:
* Parallelize CI safe checks to reduce run time [#717](https://github.com/ethyca/fidesops/pull/717)
* Add dependabot to keep dependencies up to date [#718](https://github.com/ethyca/fidesops/pull/718)

* ### Docs
### Changed
* Base64 encode passwords on frontend [#749](https://github.com/ethyca/fidesops/pull/749)

### Docs
* Updated the tutorial installation to use main in fidesdemo [#715](https://github.com/ethyca/fidesops/pull/715)

### Fixed
Expand Down
3 changes: 2 additions & 1 deletion clients/admin-ui/src/features/auth/auth.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { createApi, fetchBaseQuery } from "@reduxjs/toolkit/query/react";
import type { RootState } from "../../app/store";
import { BASE_URL, STORED_CREDENTIALS_KEY } from "../../constants";
import { addCommonHeaders } from "../common/CommonHeaders";
import { utf8ToB64 } from "../common/utils";
import { User } from "../user-management/types";
import { LoginRequest, LoginResponse } from "./types";

Expand Down Expand Up @@ -88,7 +89,7 @@ export const authApi = createApi({
query: (credentials) => ({
url: "login",
method: "POST",
body: credentials,
body: { ...credentials, password: utf8ToB64(credentials.password) },
}),
invalidatesTags: () => ["Auth"],
}),
Expand Down
5 changes: 5 additions & 0 deletions clients/admin-ui/src/features/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
export function capitalize(text: string): string {
return text.replace(/^\w/, (c) => c.toUpperCase());
}

// eslint-disable-next-line import/prefer-default-export
export function utf8ToB64(str: string): string {
return window.btoa(unescape(encodeURIComponent(str)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { RootState } from "../../app/store";
import { BASE_URL } from "../../constants";
import { selectToken } from "../auth";
import { addCommonHeaders } from "../common/CommonHeaders";
import { utf8ToB64 } from "../common/utils";
import {
User,
UserPasswordUpdate,
Expand Down Expand Up @@ -157,7 +158,10 @@ export const userApi = createApi({
query: ({ id, old_password, new_password }) => ({
url: `user/${id}/reset-password`,
method: "POST",
body: { old_password, new_password },
body: {
old_password: utf8ToB64(old_password!),
new_password: utf8ToB64(new_password!),
},
}),
invalidatesTags: ["User"],
}),
Expand Down
3 changes: 2 additions & 1 deletion create_superuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from fidesops.models.fidesops_user import FidesopsUser
from fidesops.models.fidesops_user_permissions import FidesopsUserPermissions
from fidesops.schemas.user import UserCreate
from fidesops.util.cryptographic_util import str_to_b64_str


def get_username(prompt: str) -> str:
Expand All @@ -25,7 +26,7 @@ def get_username(prompt: str) -> str:
def get_password(prompt: str) -> str:
"""Prompt the user for a password"""
password = getpass.getpass(prompt)
return password
return str_to_b64_str(password)


def get_input(prompt: str) -> str:
Expand Down
9 changes: 7 additions & 2 deletions docs/fidesops/docs/ui/user_management.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,21 @@ User permissions are managed through access tokens, which contain scopes associa

Creating a user currently provides access to all scopes.


### User Passwords
All user passwords must by Base64 encoded before creating a new user, logging in, or changing a users password. This can be done with [base64encode.org](https://www.base64encode.org/). After Base64 encoding, the password `Suitablylongwithnumber8andsymbol$` would become `U3VpdGFibHlsb25nd2l0aG51bWJlcjhhbmRzeW1ib2wk`.

### Logging in


```
POST api/v1/login
```

```json title="Request Body"
{
"username": "test_username",
"password": "Suitablylongwithnumber8andsymbol$"
"password": "U3VpdGFibHlsb25nd2l0aG51bWJlcjhhbmRzeW1ib2wk"
}
```

Expand All @@ -83,7 +88,7 @@ POST api/v1/user
```json title="Request Body"
{
"username": "new_username",
"password": "new_Suitablylongwithnumber8andsymbol$"
"password": "U3VpdGFibHlsb25nd2l0aG51bWJlcjhhbmRzeW1ib2wk"
}
```

Expand Down
29 changes: 23 additions & 6 deletions src/fidesops/schemas/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from fidesops.schemas.base_class import BaseSchema
from fidesops.schemas.oauth import AccessToken
from fidesops.util.cryptographic_util import b64_str_to_str


class UserUpdate(BaseSchema):
Expand Down Expand Up @@ -33,18 +34,19 @@ def validate_username(cls, username: str) -> str:
@validator("password")
def validate_password(cls, password: str) -> str:
"""Add some password requirements"""
if len(password) < 8:
decoded_password = b64_str_to_str(password)
if len(decoded_password) < 8:
raise ValueError("Password must have at least eight characters.")
if re.search("[0-9]", password) is None:
if re.search("[0-9]", decoded_password) is None:
raise ValueError("Password must have at least one number.")
if re.search("[A-Z]", password) is None:
if re.search("[A-Z]", decoded_password) is None:
raise ValueError("Password must have at least one capital letter.")
if re.search("[a-z]", password) is None:
if re.search("[a-z]", decoded_password) is None:
raise ValueError("Password must have at least one lowercase letter.")
if re.search(r"[\W]", password) is None:
if re.search(r"[\W]", decoded_password) is None:
raise ValueError("Password must have at least one symbol.")

return password
return decoded_password


class UserLogin(BaseSchema):
Expand All @@ -53,13 +55,28 @@ class UserLogin(BaseSchema):
username: str
password: str

@validator("password")
def validate_password(cls, password: str) -> str:
"""Convert b64 encoded password to normal string"""
return b64_str_to_str(password)


class UserPasswordReset(BaseSchema):
"""Contains both old and new passwords when resetting a password"""

old_password: str
new_password: str

@validator("old_password")
def validate_old_password(cls, old_password: str) -> str:
"""Convert b64 encoded old_password to normal string"""
return b64_str_to_str(old_password)

@validator("new_password")
def validate_new_password(cls, new_password: str) -> str:
"""Convert b64 encoded new_password to normal string"""
return b64_str_to_str(new_password)


class UserResponse(BaseSchema):
"""Response after requesting a User"""
Expand Down
12 changes: 12 additions & 0 deletions src/fidesops/util/cryptographic_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,15 @@ def bytes_to_b64_str(bytestring: bytes) -> str:
def b64_str_to_bytes(encoded_str: str) -> bytes:
"""Converts encoded string into bytes"""
return b64decode(encoded_str.encode(config.security.ENCODING))


def b64_str_to_str(encoded_str: str) -> str:
"""Converts encoded string into str"""
return b64decode(encoded_str).decode(config.security.ENCODING)


def str_to_b64_str(string: str) -> str:
"""Converts str into a utf-8 encoded string"""
return b64encode(string.encode(config.security.ENCODING)).decode(
config.security.ENCODING
)
59 changes: 39 additions & 20 deletions tests/api/v1/endpoints/test_user_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
JWE_PAYLOAD_CLIENT_ID,
JWE_PAYLOAD_SCOPES,
)
from fidesops.util.cryptographic_util import str_to_b64_str
from fidesops.util.oauth_util import extract_payload, generate_jwe
from tests.conftest import generate_auth_header_for_user

Expand Down Expand Up @@ -84,7 +85,7 @@ def test_username_exists(
) -> None:
auth_header = generate_auth_header([USER_CREATE])

body = {"username": "test_user", "password": "TestP@ssword9"}
body = {"username": "test_user", "password": str_to_b64_str("TestP@ssword9")}
user = FidesopsUser.create(db=db, data=body)

response = api_client.post(url, headers=auth_header, json=body)
Expand All @@ -103,31 +104,31 @@ def test_create_user_bad_password(
) -> None:
auth_header = generate_auth_header([USER_CREATE])

body = {"username": "test_user", "password": "short"}
body = {"username": "test_user", "password": str_to_b64_str("short")}
response = api_client.post(url, headers=auth_header, json=body)
assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code
assert (
json.loads(response.text)["detail"][0]["msg"]
== "Password must have at least eight characters."
)

body = {"username": "test_user", "password": "longerpassword"}
body = {"username": "test_user", "password": str_to_b64_str("longerpassword")}
response = api_client.post(url, headers=auth_header, json=body)
assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code
assert (
json.loads(response.text)["detail"][0]["msg"]
== "Password must have at least one number."
)

body = {"username": "test_user", "password": "longer55password"}
body = {"username": "test_user", "password": str_to_b64_str("longer55password")}
response = api_client.post(url, headers=auth_header, json=body)
assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code
assert (
json.loads(response.text)["detail"][0]["msg"]
== "Password must have at least one capital letter."
)

body = {"username": "test_user", "password": "LoNgEr55paSSworD"}
body = {"username": "test_user", "password": str_to_b64_str("LoNgEr55paSSworD")}
response = api_client.post(url, headers=auth_header, json=body)
assert HTTP_422_UNPROCESSABLE_ENTITY == response.status_code
assert (
Expand All @@ -143,7 +144,7 @@ def test_create_user(
url,
) -> None:
auth_header = generate_auth_header([USER_CREATE])
body = {"username": "test_user", "password": "TestP@ssword9"}
body = {"username": "test_user", "password": str_to_b64_str("TestP@ssword9")}

response = api_client.post(url, headers=auth_header, json=body)

Expand All @@ -164,7 +165,7 @@ def test_create_user_with_name(
auth_header = generate_auth_header([USER_CREATE])
body = {
"username": "test_user",
"password": "TestP@ssword9",
"password": str_to_b64_str("TestP@ssword9"),
"first_name": "Test",
"last_name": "User",
}
Expand Down Expand Up @@ -354,7 +355,7 @@ def test_get_users(self, api_client: TestClient, generate_auth_header, url, db):
for i in range(total_users):
body = {
"username": f"user{i}@example.com",
"password": "Password123!",
"password": str_to_b64_str("Password123!"),
"first_name": "Test",
"last_name": "User",
}
Expand Down Expand Up @@ -389,7 +390,10 @@ def test_get_filtered_users(
saved_users: List[FidesopsUser] = []
total_users = 50
for i in range(total_users):
body = {"username": f"user{i}@example.com", "password": "Password123!"}
body = {
"username": f"user{i}@example.com",
"password": str_to_b64_str("Password123!"),
}
resp = api_client.post(url, headers=create_auth_header, json=body)
assert resp.status_code == HTTP_201_CREATED
user = FidesopsUser.get_by(db, field="username", value=body["username"])
Expand Down Expand Up @@ -566,8 +570,8 @@ def test_update_different_user_password(
f"{url_no_id}/{user.id}/reset-password",
headers=auth_header,
json={
"old_password": OLD_PASSWORD,
"new_password": NEW_PASSWORD,
"old_password": str_to_b64_str(OLD_PASSWORD),
"new_password": str_to_b64_str(NEW_PASSWORD),
},
)
assert resp.status_code == HTTP_401_UNAUTHORIZED
Expand Down Expand Up @@ -599,8 +603,8 @@ def test_update_user_password_invalid(
f"{url_no_id}/{application_user.id}/reset-password",
headers=auth_header,
json={
"old_password": "mismatching password",
"new_password": NEW_PASSWORD,
"old_password": str_to_b64_str("mismatching password"),
"new_password": str_to_b64_str(NEW_PASSWORD),
},
)
assert resp.status_code == HTTP_401_UNAUTHORIZED
Expand Down Expand Up @@ -629,8 +633,8 @@ def test_update_user_password(
f"{url_no_id}/{application_user.id}/reset-password",
headers=auth_header,
json={
"old_password": OLD_PASSWORD,
"new_password": NEW_PASSWORD,
"old_password": str_to_b64_str(OLD_PASSWORD),
"new_password": str_to_b64_str(NEW_PASSWORD),
},
)
assert resp.status_code == HTTP_200_OK
Expand All @@ -646,19 +650,28 @@ def url(self, oauth_client: ClientDetail) -> str:
return V1_URL_PREFIX + LOGIN

def test_user_does_not_exist(self, db, url, api_client):
body = {"username": "does not exist", "password": "idonotknowmypassword"}
body = {
"username": "does not exist",
"password": str_to_b64_str("idonotknowmypassword"),
}
response = api_client.post(url, headers={}, json=body)
assert response.status_code == HTTP_404_NOT_FOUND

def test_bad_login(self, db, url, user, api_client):
body = {"username": user.username, "password": "idonotknowmypassword"}
body = {
"username": user.username,
"password": str_to_b64_str("idonotknowmypassword"),
}
response = api_client.post(url, headers={}, json=body)
assert response.status_code == HTTP_403_FORBIDDEN

def test_login_creates_client(self, db, url, user, api_client):
# Delete existing client for test purposes
user.client.delete(db)
body = {"username": user.username, "password": "TESTdcnG@wzJeu0&%3Qe2fGo7"}
body = {
"username": user.username,
"password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"),
}

assert user.client is None # client does not exist
assert user.permissions is not None
Expand All @@ -681,7 +694,10 @@ def test_login_creates_client(self, db, url, user, api_client):
user.client.delete(db)

def test_login_updates_last_login_date(self, db, url, user, api_client):
body = {"username": user.username, "password": "TESTdcnG@wzJeu0&%3Qe2fGo7"}
body = {
"username": user.username,
"password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"),
}

response = api_client.post(url, headers={}, json=body)
assert response.status_code == HTTP_200_OK
Expand All @@ -690,7 +706,10 @@ def test_login_updates_last_login_date(self, db, url, user, api_client):
assert user.last_login_at is not None

def test_login_uses_existing_client(self, db, url, user, api_client):
body = {"username": user.username, "password": "TESTdcnG@wzJeu0&%3Qe2fGo7"}
body = {
"username": user.username,
"password": str_to_b64_str("TESTdcnG@wzJeu0&%3Qe2fGo7"),
}

existing_client_id = user.client.id
user.client.scopes = [PRIVACY_REQUEST_READ]
Expand Down
Loading