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

Commit

Permalink
Password Hashing update (#749)
Browse files Browse the repository at this point in the history
  • Loading branch information
TheAndrewJackson authored Jun 29, 2022
1 parent b976bc3 commit 2ae99fe
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 37 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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)
* Added a page on how to use the datastore UI [#742](https://github.com/ethyca/fidesops/pull/742)

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

0 comments on commit 2ae99fe

Please sign in to comment.