Skip to content

Commit

Permalink
[CPNHUB-248] Multi auth handling changes (#128)
Browse files Browse the repository at this point in the history
* Send PasswordReset to server if user not found in firebase

* Set company auth_provider from mgmt_api

* Dont send CEM updates for non google accounts

* Add AUTH_PROVIDERS for Google & Azure

* Fix email_sent check used while developing

* fix(build): Upgrade to latest eve-elastic

* add/fix tests

* fix lint

* remove example test

* fix(ci): Improve npm install times
  • Loading branch information
MarkLark86 authored Aug 30, 2023
1 parent 359e3fd commit 5dd08b3
Show file tree
Hide file tree
Showing 13 changed files with 185 additions and 37 deletions.
8 changes: 5 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ jobs:
working-directory: client

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3

- uses: actions/setup-node@v1
- uses: actions/setup-node@v3
with:
node-version: '14.x'

- run: git config --global url."https://git@".insteadOf git://

- name: install
run: npm install
run: npm ci || npm install

- name: build
run: npm run build
12 changes: 10 additions & 2 deletions client/src/reset-password.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const form = document.getElementById('reset-password-form');
const url = new URL(window.nextUrl);
const params = new URLSearchParams(url.search);
const sendButton = document.getElementById('send-email');
const emailSentCheckbox = document.getElementById('email_sent_checkbox');

form.onsubmit = (event) => {
event.preventDefault();
Expand All @@ -22,12 +23,19 @@ form.onsubmit = (event) => {
sendButton.disabled = true;
sendPasswordResetEmail(auth, email, {url: url.toString()})
.then(() => {
// set `email_sent` to true, so server knows password reset was handled by firebase
emailSentCheckbox.checked = true;
form.submit();
return true;
})
.catch((reason) => {
console.error(reason);
sendButton.disabled = false; // allow another request if there was an error
if (reason.code === 'auth/user-not-found') {
// User not registered with OAuth, try attempting normal password reset
form.submit();
} else {
console.error(reason);
sendButton.disabled = false; // allow another request if there was an error
}
});

return false;
Expand Down
44 changes: 40 additions & 4 deletions server/cp/auth.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
from typing import Optional
import flask
import logging
import google.oauth2.id_token

from flask_babel import gettext
from google.auth.transport import requests
from newsroom.auth.utils import sign_user_by_email

from newsroom.types import AuthProviderType
from newsroom.auth import get_company, get_user_by_email
from newsroom.auth.utils import sign_user_by_email, get_company_auth_provider, send_token
from newsroom.auth.views import logout as _logout

from .password_reset_form import PasswordResetForm

TIMEOUT = 5

logger = logging.getLogger(__name__)
Expand All @@ -31,7 +37,7 @@ def token():
return flask.redirect(flask.url_for("auth.login", token_error=1))

email = claims["email"]
return sign_user_by_email(email)
return sign_user_by_email(email, validate_login_attempt=True)

return flask.redirect(flask.url_for("auth.login"))

Expand All @@ -49,6 +55,36 @@ def reset_password_confirmation():

@blueprint.route("/cp_reset_password", methods=["GET", "POST"])
def reset_password():
if flask.request.method == "POST":
form = PasswordResetForm()

def render_reset_page(error_str: Optional[str] = None):
if error_str is not None:
flask.flash(error_str, "danger")

return flask.render_template("cp_reset_password.html", form=form)

if form.validate_on_submit():
if form.email_sent.data is True:
# Password reset was handled by firebase from the front-end
return flask.redirect(flask.url_for("cp.auth.reset_password_confirmation"))

# Password reset was not handled in the front-end, continue with standard password reset procedure
user = get_user_by_email(form.email.data)
if not user:
return render_reset_page(gettext("User not found"))
elif not user.get("is_enabled"):
return render_reset_page(gettext("User not enabled"))

company = get_company(user)
auth_provider = get_company_auth_provider(company)

if auth_provider["auth_type"] != AuthProviderType.PASSWORD.value:
return render_reset_page(gettext("Password reset for your account is not supported through Newshub"))

# Send standard Newshub reset password email
if not send_token(user, "reset_password"):
return render_reset_page(gettext("An error occurred while sending reset password email"))

return flask.redirect(flask.url_for("cp.auth.reset_password_confirmation"))
return flask.render_template("cp_reset_password.html")

return render_reset_page()
9 changes: 9 additions & 0 deletions server/cp/mgmt_api_signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from newsroom.signals import company_create


def on_company_create(sender, company, **kwargs):
company.setdefault("auth_provider", "gip")


def init_app(app):
company_create.connect(on_company_create)
9 changes: 9 additions & 0 deletions server/cp/password_reset_form.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from flask_wtf import FlaskForm
from wtforms import StringField, BooleanField
from wtforms.validators import DataRequired, Length, Email
from flask_babel import lazy_gettext


class PasswordResetForm(FlaskForm):
email = StringField(lazy_gettext("Email"), validators=[DataRequired(), Length(1, 64), Email()])
email_sent = BooleanField("email_sent", validators=[])
29 changes: 23 additions & 6 deletions server/cp/signals.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from typing import Optional
from newsroom.types import User, Company
import cp

from superdesk import get_resource_service
from newsroom.signals import publish_item, user_created, user_updated, user_deleted, push

from cp.cem import send_notification
Expand Down Expand Up @@ -33,25 +36,39 @@ def copy_correction_to_body_html(item):


def on_user_created(sender, user, **kwargs):
send_notification("new user", user)
if user_auth_is_gip(user):
send_notification("new user", user)


def on_user_updated(sender, user, updates=None, **kwargs):
if updates and updates.get("password"):
send_notification("password change", user)
else:
send_notification("update user", user)
if user_auth_is_gip(user):
if updates and updates.get("password"):
send_notification("password change", user)
else:
send_notification("update user", user)


def on_user_deleted(sender, user, **kwargs):
send_notification("delete user", user)
if user_auth_is_gip(user):
send_notification("delete user", user)


def on_push(sender, item, **kwargs):
if item.get("language"):
item["language"] = fix_language(item["language"])


def user_auth_is_gip(user: User) -> bool:
if not user.get("company"):
return False

company: Optional[Company] = get_resource_service("companies").find_one(req=None, _id=user["company"])
if not company:
return False

return company.get("auth_provider") == "gip"


def init_app(app):
publish_item.connect(on_publish_item)
user_created.connect(on_user_created)
Expand Down
32 changes: 18 additions & 14 deletions server/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# This file is autogenerated by pip-compile with Python 3.10
# This file is autogenerated by pip-compile with Python 3.8
# by the following command:
#
# pip-compile requirements.in
Expand All @@ -17,7 +17,7 @@ asn1crypto==1.5.1
# oscrypto
# pyhanko
# pyhanko-certvalidator
async-timeout==4.0.2
async-timeout==4.0.3
# via redis
authlib==0.14.3
# via superdesk-core
Expand All @@ -34,9 +34,9 @@ blinker==1.4
# raven
# sentry-sdk
# superdesk-core
boto3==1.28.14
boto3==1.28.38
# via superdesk-core
botocore==1.31.14
botocore==1.31.38
# via
# boto3
# s3transfer
Expand All @@ -48,7 +48,7 @@ cachetools==5.3.1
# google-auth
celery[redis]==5.2.7
# via superdesk-core
cerberus==1.3.4
cerberus==1.3.5
# via
# eve
# superdesk-core
Expand All @@ -68,7 +68,7 @@ charset-normalizer==3.2.0
# via requests
ciso8601==1.0.8
# via eve-elastic
click==8.1.6
click==8.1.7
# via
# celery
# click-didyoumean
Expand All @@ -86,7 +86,7 @@ click-repl==0.3.0
# via celery
croniter==0.3.37
# via superdesk-core
cryptography==41.0.2
cryptography==41.0.3
# via
# authlib
# jwcrypto
Expand All @@ -102,15 +102,15 @@ deprecated==1.2.14
# limits
draftjs-exporter[lxml]==2.1.7
# via superdesk-core
ecs-logging==2.0.2
ecs-logging==2.1.0
# via elastic-apm
elastic-apm[flask]==6.18.0
# via superdesk-core
elasticsearch==7.13.4
# via eve-elastic
eve==1.1.2
# via superdesk-core
eve-elastic==7.3.1
eve-elastic==7.3.2
# via
# newsroom-core
# superdesk-core
Expand Down Expand Up @@ -172,7 +172,7 @@ icalendar==4.0.9
# via superdesk-planning
idna==3.4
# via requests
importlib-resources==6.0.0
importlib-resources==6.0.1
# via limits
isodate==0.6.1
# via python3-saml
Expand Down Expand Up @@ -262,11 +262,11 @@ pymongo==3.11.4
# flask-pymongo
# mongolock
# superdesk-core
pyparsing==3.1.0
pyparsing==3.1.1
# via
# httplib2
# pyrtf3
pypdf==3.13.0
pypdf==3.15.4
# via xhtml2pdf
pypng==0.20220715.0
# via qrcode
Expand All @@ -291,6 +291,7 @@ python3-saml==1.15.0
# via newsroom-core
pytz==2023.3
# via
# babel
# celery
# eve-elastic
# flask-babel
Expand Down Expand Up @@ -330,7 +331,7 @@ rsa==4.9
# via
# google-auth
# oauth2client
s3transfer==0.6.1
s3transfer==0.6.2
# via boto3
sentry-sdk[flask]==1.5.12
# via newsroom-core
Expand Down Expand Up @@ -362,6 +363,7 @@ tinycss2==1.2.1
typing-extensions==4.7.1
# via
# limits
# pypdf
# qrcode
# superdesk-core
tzlocal==2.1
Expand All @@ -370,7 +372,7 @@ tzlocal==2.1
# superdesk-core
unidecode==0.4.21
# via superdesk-core
uritools==4.0.1
uritools==4.0.2
# via pyhanko-certvalidator
urllib3==1.25.11
# via
Expand Down Expand Up @@ -410,6 +412,8 @@ xhtml2pdf==0.2.11
# via newsroom-core
xmlsec==1.3.13
# via python3-saml
zipp==3.16.2
# via importlib-resources

# The following packages are considered to be unsafe in a requirements file:
# setuptools
12 changes: 12 additions & 0 deletions server/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
BLUEPRINTS as DEFAULT_BLUEPRINTS,
CELERY_BEAT_SCHEDULE as DEFAULT_CELERY_BEAT_SCHEDULE,
CLIENT_URL,
AUTH_PROVIDERS,
)
from newsroom.types import AuthProviderType


SERVER_PATH = pathlib.Path(__file__).resolve().parent
Expand Down Expand Up @@ -276,3 +278,13 @@

AGENDA_SECTION = lazy_gettext("Calendar")
SAVED_SECTION = lazy_gettext("Bookmarks")

AUTH_PROVIDERS.extend([{
"_id": "gip",
"name": lazy_gettext("Google"),
"auth_type": AuthProviderType.GOOGLE_OAUTH.value,
}, {
"_id": "azure",
"name": lazy_gettext("Azure"),
"auth_type": AuthProviderType.SAML.value,
}])
4 changes: 4 additions & 0 deletions server/settings_mgmtapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
ABS_PATH = Path(__file__).resolve().parent

MGMT_API_ENABLED = True

INSTALLED_APPS = [
"cp.mgmt_api_signals",
]
5 changes: 0 additions & 5 deletions server/tests/test_example.py

This file was deleted.

Loading

0 comments on commit 5dd08b3

Please sign in to comment.