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

Check for auth on importing linkedin data #4429

Merged
merged 4 commits into from
May 29, 2019
Merged

Conversation

danlipert
Copy link
Contributor

@danlipert danlipert commented May 17, 2019

Description

This PR checks if the user is authenticated before accepting the updated data for their job opportunity profile.

Refers/Fixes

This issue was referred to us by a security researcher.

Testing

After this change I am still able to update my own profile. If I change to change the profile handle in the URL I now receive a bad request error

@danlipert danlipert requested a review from a team May 17, 2019 12:46
@codecov
Copy link

codecov bot commented May 17, 2019

Codecov Report

Merging #4429 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4429      +/-   ##
==========================================
- Coverage   30.13%   30.12%   -0.01%     
==========================================
  Files         208      208              
  Lines       16787    16791       +4     
  Branches     2254     2256       +2     
==========================================
  Hits         5058     5058              
- Misses      11535    11539       +4     
  Partials      194      194
Impacted Files Coverage Δ
app/dashboard/views.py 14.19% <0%> (-0.05%) ⬇️

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 4ec26ba...b463cb9. Read the comment docs.

@codecov
Copy link

codecov bot commented May 17, 2019

Codecov Report

Merging #4429 into master will increase coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4429      +/-   ##
==========================================
+ Coverage   30.02%   30.02%   +<.01%     
==========================================
  Files         209      209              
  Lines       16881    16884       +3     
  Branches     2276     2277       +1     
==========================================
+ Hits         5069     5070       +1     
- Misses      11616    11618       +2     
  Partials      196      196
Impacted Files Coverage Δ
app/dashboard/views.py 14.3% <33.33%> (+0.04%) ⬆️

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 3249e6f...2e3a242. Read the comment docs.

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

@danlipert we could make this bit cleaner by simply using @login_required decorator
would be cleaner. We've used in for other views!

@danlipert
Copy link
Contributor Author

@thelostone-mc I've added the @login_required decorator - thanks for the suggestion!

@danlipert danlipert requested review from SaptakS and octavioamu May 29, 2019 11:30
@thelostone-mc thelostone-mc merged commit 50b5b81 into master May 29, 2019
@thelostone-mc thelostone-mc deleted the job-opp-auth branch July 4, 2019 14:53
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.

2 participants