Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compliance - blocked country and user list #5602

Merged
merged 4 commits into from
Jan 8, 2020

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Dec 5, 2019

Description

from a complilance perspective.. we do not want to do business with those in OFAC countries or on the OFAC list. this PR takes an initial stab at making sure those users cannot login to the platform

Refers/Fixes

email thread with product & legal

Testing

tested locally

@codecov
Copy link

codecov bot commented Dec 5, 2019

Codecov Report

Merging #5602 into master will increase coverage by 0.13%.
The diff coverage is 26.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5602      +/-   ##
==========================================
+ Coverage   30.24%   30.38%   +0.13%     
==========================================
  Files         247      253       +6     
  Lines       21165    23546    +2381     
  Branches     3065     3745     +680     
==========================================
+ Hits         6402     7154     +752     
- Misses      14487    16029    +1542     
- Partials      276      363      +87
Impacted Files Coverage Δ
app/app/settings.py 80.2% <ø> (ø) ⬆️
app/compliance/views.py 0% <0%> (ø)
app/compliance/apps.py 0% <0%> (ø)
...liance/management/commands/pull_compliance_list.py 0% <0%> (ø)
app/compliance/admin.py 100% <100%> (ø)
app/dashboard/utils.py 43.8% <3.84%> (+0.65%) ⬆️
app/compliance/models.py 88.88% <88.88%> (ø)
app/app/urls.py 84.74% <0%> (-5.46%) ⬇️
app/grants/views.py 13.64% <0%> (-1.84%) ⬇️
app/dashboard/tip_views.py 17.32% <0%> (-1.39%) ⬇️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d28eb16...113a984. Read the comment docs.

@octavioamu
Copy link
Contributor

As we use github login and they already do this. Do we need to recheck?

@owocki
Copy link
Contributor Author

owocki commented Dec 5, 2019

thats a good question to ask legal on our call with them next week.

cc @frankchen07

Copy link
Contributor

@danlipert danlipert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty legit to me - we'll likely encounter some false positives along the way so may want to add some whitelist logic as well

first_name = self.user.first_name
last_name = self.user.last_name
full_name = '{first_name} {last_name}'
is_on_banned_user_list = Entitty.objects.filter(fullName__icontains=full_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owocki
Copy link
Contributor Author

owocki commented Dec 11, 2019

sounds like per the legal call its a good idea to have our own checking in place

@owocki
Copy link
Contributor Author

owocki commented Dec 11, 2019

i turned around the code review..

@danlipert
Copy link
Contributor

i turned around the code review..

@owocki is this you saying that its ready for final review? I see there is a note in the description that says it shouldn't be merged

@owocki
Copy link
Contributor Author

owocki commented Dec 16, 2019

@danlipert just did some final testing + submitted it (removing the hedge from the desc)

app/dashboard/utils.py Outdated Show resolved Hide resolved

def insert_entities():
# clear existing table
Entity.objects.all().delete()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to ever do any kind of tracking / recording of compliance related service denial or anything like that we'll have to not rewrite this table over and over - a premature optimization for now but lets all make sure we are aware of this for the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it; @alexvotofuture should we keep a record of OFAC denials? would it be handy to have to show regulatory compliance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets punt to a v2

@owocki
Copy link
Contributor Author

owocki commented Dec 18, 2019

@danlipert turned around ur feedback

Entity.objects.all().delete()

# pull data
url = 'https://www.treasury.gov/ofac/downloads/consolidated/consolidated.xml'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who gave this this btw ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont understand the question?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants