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.

Sign up for GitHub

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

Jump to bottom

Improve EPP session management #700

Closed
artur-intech opened this issue Feb 6, 2018 · 8 comments
Closed

Improve EPP session management #700

artur-intech opened this issue Feb 6, 2018 · 8 comments
Assignees
@vohmar
Labels
blocked enhancement

Comments

@artur-intech
Copy link
Contributor

artur-intech commented Feb 6, 2018 •
edited
Loading

  • Add database constraints
  • Refactor EPPSession model, EPP::SessionsController, EppController
  • Refactor database structure
  • Add basic EPP login/logout tests
@artur-intech artur-intech self-assigned this Feb 6, 2018
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Do not allow NULL in epp_sessions.session_id
34f7820 
#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Convert spec to test
cdb0187 
#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Do not use factory
a757a92 
#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Remove factory
4163ee9 
#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Disallow blank EppSession#session_id
1c560b1 
#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Remove unneeded if statement
d5b9606 
epp_session.session_id is always present

#700
artur-intech pushed a commit that referenced this issue Feb 6, 2018
Remove test logic
82c74a4 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Add epp_sessions.user_id
ba341ee 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Decompose EppSession#data
940613a 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Fix invalid factory
ad1f5e6 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Fix EPP login in specs
278ae07 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Remove rspec helper
2dcf0f5 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Rename migration
c5f8c7e 
#700
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Migrate epp_sessions.data to user_id
38f967a 
#700
@artur-intech artur-intech mentioned this issue Feb 7, 2018
Closed
artur-intech pushed a commit that referenced this issue Feb 7, 2018
Quote values
114361f 
#700
artur-intech pushed a commit that referenced this issue Feb 12, 2018
Create new EPP session on login explicitly
b11306e 
#700
artur-intech pushed a commit that referenced this issue Feb 12, 2018
Remove extra attribute
fa6edab 
Registrar can be queried from user

#700
artur-intech pushed a commit that referenced this issue Feb 12, 2018
Replace index with constraint
5a101dd 
#700
artur-intech pushed a commit that referenced this issue Feb 12, 2018
Add EPP session tests
e2c90cd 
#700
@teadur
Copy link
Contributor

teadur commented Feb 13, 2018

Please consider env['HTTP_COOKIE'] as session source if cookies doesnot give any extra value.

artur-intech pushed a commit that referenced this issue Feb 13, 2018
Improve readability
40c1238 
#700
artur-intech pushed a commit that referenced this issue Feb 13, 2018
Ensure unique EPP session id
4ec4e50 
#700
artur-intech pushed a commit that referenced this issue Feb 13, 2018
Remove invalid association
3c274ba 
Completes fa6edab

#700
@artur-intech
Copy link
Contributor Author

artur-intech commented Feb 13, 2018 •
edited
Loading

@vohmar @ratM1n Special attention to db/migrate/20180207071528_extract_user_id_from_epp_sessions_data.rb It extracts user id from serialised data attribute. Double check that it converts existing data correctly (on staging, test-reg).

Marshal.load(::Base64.decode64(EppSession.first.data_before_type_cast))[:api_user_id] in rails c allows to see actual contents of #data, where EppSession.first is the session in question.

Take into account that after all migrations run, there will be no data and registrar_id anymore.

artur-intech pushed a commit that referenced this issue Feb 13, 2018
Add NOT NULL constraint
1f66f13 
#700
artur-intech pushed a commit that referenced this issue Feb 13, 2018
Add database constraint test
fc6a2df 
Having session_id constraints at the database level is crucial

#700
artur-intech pushed a commit that referenced this issue Feb 13, 2018
Do not update EPP session updated_at if not authenticated
cd037f7 
#700
artur-intech pushed a commit that referenced this issue Feb 13, 2018
Simplify method
0c5284e 
#700
artur-intech pushed a commit that referenced this issue Feb 14, 2018
Hide method
66b03e6 
#700
@artur-intech artur-intech changed the title Improve EPP session model validation and DB constraints Improve EPP session management Feb 14, 2018
artur-intech pushed a commit that referenced this issue Feb 14, 2018
Extract method
d430092 
#700
artur-intech pushed a commit that referenced this issue Feb 14, 2018
Remove environment dependency
fc02e4a 
#700
@artur-intech
Copy link
Contributor Author

artur-intech commented Feb 14, 2018 •
edited
Loading

Test:

  • EPP login/logout
  • EPP session limit
  • EPP logs in admin

Multiple users from different registrars must exist.

artur-intech pushed a commit that referenced this issue Feb 14, 2018
Hide method
93055ac 
#700
artur-intech pushed a commit that referenced this issue Feb 14, 2018
Refactor EPP session limit
9b4aa47 
#700
artur-intech pushed a commit that referenced this issue Feb 14, 2018
Reorganize tests
46e8596 
#700
@artur-intech artur-intech assigned vohmar and unassigned artur-intech Feb 14, 2018
This was referenced Feb 15, 2018
Closed
Closed
@vohmar
Copy link
Contributor

vohmar commented Feb 19, 2018 •
edited
Loading

  • session limit does not work - successfully created 10 simultaneous sessions under a single registrar
  • epp login/logout works
  • epp logs in admin seem OK

@vohmar vohmar assigned artur-intech and unassigned vohmar Feb 19, 2018
@artur-intech
Copy link
Contributor Author

How can I reproduce № 1?

@artur-intech
Copy link
Contributor Author

artur-intech commented Feb 19, 2018 •
edited
Loading

As far as I remember, we decided to postpone this issue anyway. Probably #711 should be done first. Otherwise nobody is allowed to login.

@teadur
Copy link
Contributor

teadur commented Feb 19, 2018

So the #710 shouldnot be reviewed before #711 is finished ?

@teadur teadur added the blocked label Feb 19, 2018
@teadur teadur mentioned this issue Feb 19, 2018
Merged
@artur-intech
Copy link
Contributor Author

@teadur That's up to @vohmar to decide. By "Test EPP session limit" I meant that exactly the same behaviour as in master branch should remain in place. I only did some refactoring concerning that part.

@artur-intech artur-intech removed their assignment Feb 19, 2018
@vohmar vohmar closed this as completed Feb 23, 2018
@artur-intech artur-intech mentioned this issue Apr 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vohmar vohmar

Labels
blocked enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@teadur @vohmar @artur-intech