Add support for OAuth/OpenID Connect #1300

Vad1mo · 2017-06-10T11:41:40Z

This PR adds OAuth Social Logins via omniauth.

OAuth Providers:

OpenID Connect
Google
GitHub
Bitbucket
GitLab

Closes #645
Foundation for #527

Features:

During his first signup the user can change/set his username
Allow to restrict signups to a specific domain. From user with a @organisation.com email address.
GitHub/GitLab, Option to restrict signups to users that are part of an organisation and team.
Google, Option to restrict signups to users in a domain.

Social Login Buttons:

Option for the user the choose/modify the Username before the account is finally created.

New configuration Options

# OAuth support.
oauth:
  # If enabled, users can authenticate with their Google Account.
  google_oauth2:
    enabled: false
    # Credentials. Details on https://developers.google.com/identity/protocols/OpenIDConnect
    id: ""
    secret: ""
    # If a domain (e.g. mycompany.com) is set, then only signups with email from this domain are allowed.
    domain: ""
    options:
      # G Suite domain. If set, then only members of the domain can sign in/up.
      # If it's empty then any google users con sign in/up.
      hd: ""
  
  # OpenID authentication support. If enabled, then users can authenticate with OpenID/Connect
  open_id:
    enabled: false
    # Optional. If identifier set then user redirect to the OpenID provider.
    # If not, then user is asked for identifier before redirect.
    # Example https://openid.stackexchange.com
    identifier: ""
    # If a domain (e.g. mycompany.com) is set, then only signups with email from this domain are allowed.
    domain: ""

  # Github authentication support.
  github:
    enabled: false
    # Application credentials.
    key: ""
    secret: ""
    # Only members of organization's team can sign in/up with Github.
    organization: ""
    team: ""
     # If a domain (e.g. mycompany.com) is set, then only signups with email from this domain are allowed.
    domain: ""

  # Gitlab authentication support.
  gitlab:
    enabled: false
    id: ""
    secret: ""
    # Only member of the group can sign in/up with Gitlab.
    group: ""
     # If a domain (e.g. mycompany.com) is set, then only signups with email from this domain are allowed.
    domain: ""

mssola · 2017-08-01T10:08:34Z

README.md

@@ -49,7 +49,7 @@ setups in which you can deploy Portus.
 Some highlights:

 - [Synchronization with your private registry in order to fetch which images and tags are available](http://port.us.org/features/1_Synchronizing-the-Registry-and-Portus.html).
- [LDAP user authentication](http://port.us.org/features/2_LDAP-support.html).
+- [LDAP, OAuth, OpenID-Connect user authentication](http://port.us.org/features/2_LDAP-support.html).


Note that the link is for LDAP only. We can write another page for other services later. In the meantime, move the OAuth, OpenID-Connect into another point (without link then).

mssola · 2017-08-01T10:36:49Z

app/controllers/auth/omniauth_callbacks_controller.rb

+
+  def gitlab
+    auth_user
+  end


Instead of:

def google_oauth2 auth_user end def open_id auth_user end def github auth_user end def gitlab auth_user end

Do the following:

alias_method :google_oauth2, :auth_user alias_method :open_id, :auth_user alias_method :github, :auth_user alias_method :gitlab, :auth_user

mssola · 2017-08-01T10:37:07Z

app/controllers/auth/omniauth_callbacks_controller.rb

+
+  private
+
+  def auth_user


Add documentation

mssola · 2017-08-01T10:45:32Z

app/controllers/auth/omniauth_registrations_controller.rb

+  def new
+    return new_user_session_url unless (data = oauth_data)
+    @user = User.new username: data["info"]["username"], display_name: data["info"]["name"]
+    @user.suggest_username data["info"]


I'd prefer something like this:

def new data = oauth_data if data redirect_to new_user_session_url else @user = User.new username: data["info"]["username"], display_name: data["info"]["name"] @user.suggest_username data["info"] end end

Mainly because I think doing this (data = oauth_data) is super ugly 😀. Plus, since it's a short method, we don't have to write it with an early return when a good ol' if/else statement would do it.

Also, since suggest_username might return a generated username, make sure to show a flashy message being more explicit with it. Like "Successfully registered as myusername!"

mssola · 2017-08-01T10:46:44Z

app/models/user.rb

@@ -213,6 +217,27 @@ def update_activities!(owner)
                    parameters: { username: username }
  end

+  def self.create_from_oauth(params, data)


Could you add documentation on this ? Things like where does this data come from, the expected values, etc.

mssola · 2017-08-01T10:47:43Z

app/models/user.rb

+    User.create params
+  end
+
+  # If usernsme exists then try variant username + "_nn".


Typo: usernsme -> username.

mssola · 2017-08-01T11:02:54Z

lib/portus/config.rb

        if objs.length == 2
          return false if !self[objs[0]][objs[1]] || self[objs[0]][objs[1]].empty?
          self[objs[0]][objs[1]]["enabled"].eql?(true)
        else
-          return false if !self[feature] || self[feature].empty?
+          # return false if !self[feature] || self[feature].empty?


Can't you just remove this ?

mssola · 2017-08-01T12:01:01Z

Overall it looks good to me. Just fix my issues. Besides this, I'm adding @vitoravelino as a reviewer and I'll proceed to further test this locally.

mssola · 2017-08-01T14:15:04Z

You may also want to document which callback to be used for each provider. I've tested Github and Gitlab. A couple of comments:

Github: they don't use the same terminology (key is Client ID and secret is Client secret). You should document this as well, since for some users this might not be obvious (it is, but let's play safe...).
Gitlab: you should document the scopes needed for this to work. Plus, I couldn't test this with my company's Gitlab instance. Could you add support for this ?

vitoravelino · 2017-08-10T17:38:26Z

app/views/devise/sessions/new.html.slim

        - if show_first_user_alert?
          .alert.alert-info
            strong Note:
            |  The first user to be created will have admin permissions !
+
+        - elsif APP_CONFIG["oauth"] && APP_CONFIG["oauth"].find_all {|k, v| v["enabled"]}.size > 0


Extract this condition to a helper

vitoravelino · 2017-08-10T17:38:57Z

app/views/devise/sessions/new.html.slim

        - if show_first_user_alert?
          .alert.alert-info
            strong Note:
            |  The first user to be created will have admin permissions !
+
+        - elsif APP_CONFIG["oauth"] && APP_CONFIG["oauth"].find_all {|k, v| v["enabled"]}.size > 0
+          .row


Extract the whole social login to its own partial.

vitoravelino · 2017-08-10T17:40:21Z

app/assets/stylesheets/includes/forms.scss

@@ -36,6 +36,20 @@ section.sign-up {
  .btn-link {
    color: $white
  }
+  h1, h2, h3, h4, h5 {


I would prefer to change this to something like .social-login-title and add it to the html element.

vitoravelino · 2017-08-10T17:41:03Z

app/assets/stylesheets/includes/forms.scss

+    margin-bottom: 5px;
+    border-radius: 4px;
+  }
+  p {


Same apply to this. Be more specific.

Btw, can you tell me where this is being applied? I couldn't find it.

will remove it

vitoravelino · 2017-08-10T17:51:51Z

app/assets/stylesheets/includes/forms.scss

+  h1, h2, h3, h4, h5 {
+    color: $white;
+    padding: 6px 12px;
+    font-family: arial;


No need of font-family.

vitoravelino · 2017-08-10T17:59:22Z

app/assets/stylesheets/includes/forms.scss

@@ -36,6 +36,20 @@ section.sign-up {
  .btn-link {
    color: $white
  }
+  h1, h2, h3, h4, h5 {
+    color: $white;
+    padding: 6px 12px;


I usually prefer to use margin instead of padding for spacing titles. In this case, I would guess margin-top 30px (20px default + 10px that you were already doing with padding) and margin-bottom 15px. That's my personal preference. What do you think?

vitoravelino · 2017-08-10T18:22:51Z

You may also want to document which callback to be used for each provider.

Agreed. 👍

vitoravelino · 2017-08-10T18:30:26Z

packaging/suse/patches/0_change_versions.gem.patch

@@ -7,7 +7,7 @@ index 5a5b38d20e7e..309f537a8fa2 100644

 # If the deployment is done through Puma, include it in the bundle.
 -gem "puma", "~> 3.7.0" if ENV["PORTUS_PUMA_DEPLOYMENT"] == "yes" || !packaging?
-+gem "puma", "~> 2.16.0" if ENV["PORTUS_PUMA_DEPLOYMENT"] == "yes" || !packaging?
+gem "puma", "= 2.16.0" if ENV["PORTUS_PUMA_DEPLOYMENT"] == "yes" || !packaging?


@mssola Is this change necessary? Just wondering...

I don't know where it comes from. Will try to find out.

It comes from a rebase I think. I introduced this change in a commit that happened when this was being reviewed-developed.

vitoravelino · 2017-08-10T18:30:31Z

packaging/suse/patches/0_change_versions.gem.patch

@@ -120,7 +120,7 @@ index 329a6d22d549..d584cc4a122f 100644
   pry-rails
   public_activity
 -  puma (~> 3.7.0)
-+  puma (~> 2.16.0)
+  puma (= 2.16.0)


vitoravelino · 2017-08-10T18:52:17Z

app/views/auth/omniauth_registrations/new.html.slim

+      = form_for @user, url: users_oauth_url do |f|
+        = f.text_field :username, class: 'input form-control input-lg first', placeholder: 'Username', autofocus: true, required: true
+        = f.text_field :display_name, class: 'input form-control input-lg last', placeholder: 'Display name'
+        = f.button 'Create account', class: 'classbutton btn btn-primary btn-block btn-lg'


Add .fa-check icon as in the standard registration page.

vitoravelino · 2017-08-10T19:00:55Z

app/views/auth/omniauth_registrations/new.html.slim

+      = image_tag 'layout/portus-logo-login-page.png', class: 'login-picture'
+      .row
+        .col-sm-12
+          h5.create-new-account Define your Username and Display Name


I would suggest to change to change this to p and ignore the .col-sm-12 and .row, going right below the image_tag.

You can also associate it with a class and style it as you might need.

vitoravelino · 2017-08-10T19:03:52Z

I'm not sure if this is a bug in the omniauth-github gem, but the suggested username was the beginning of my email instead of my github username.

vitoravelino

I've made a few comments... If you have any question, please feel free to reach me out, ok? I'll be more than happy to help you with it.

Thanks a lot for the PR, a really great feature! 👍

andrew2net · 2017-08-11T17:35:28Z

@vitoravelino suggesting username is not omniauth-github function. We suggest username based on email because not all providers have a username. For example, Open ID doesn't provide a username.

It was just weird to have contact appearing as the suggested username. I would prefer to leave the username empty and just the full name filled. Is that possible? What do you think, @mssola?

Of course, it possible to leave the username empty, but since the feature is @Vad1mo suggestion, I think we should wait for his comment.
May be it would better leave empty the field when a provider doesn't supply username, and fill it when username supplied.

Vad1mo · 2017-08-12T22:05:12Z

Since the username can't be changed once it is set the user need to choose hist uername quite wisely as he is going to use it for each image push/pull.

In the first iteration of OAuth there wasn't even an option to set username and password. Thats why we decided to give the user the option to set its username when it is possible.

From the usability and uniformity point of view this process would fit best:

OAuth provider delivers a username. Then the input field is prefilled with that username.
OAuth provider does not provide username. Then the input field is prefilled with email prefix.
In many cases this might be exactly what the user wants. Think of organisations using the registry.

Leaving the username out in the second case, is personally OK for me, however it would break the uniformity of the flow.

Vad1mo · 2017-08-24T11:57:36Z

@mssola @vitoravelino, what changes should we implement for this PR?

vitoravelino · 2017-09-06T17:40:08Z

config/config.yml

+  github:
+    enabled: false
+    # Application credentials.
+    key: ""


As @mssola mentioned a few comments ago, it would be nice to rename this to client_id and client_secret below to avoid confusion.

vitoravelino · 2017-09-06T17:40:31Z

config/config.yml

+  # Gitlab authentication support.
+  gitlab:
+    enabled: false
+    id: ""


Here could be application_id

vitoravelino · 2017-09-06T17:46:24Z

Missing space between icon and text:

vitoravelino · 2017-09-06T17:49:04Z

I'm not sure if I missed this somewhere but it would be nice to have somewhere documented which callback url should be used for each service. Also, which are the minimum permissions to be selected to make the integration work (gitlab and bitbucket forces you to choose something).

vitoravelino

After implementing those changes, it's a huge 👍 from myself.

andrew2net · 2017-09-06T18:14:44Z

@vitoravelino I have no image there callback URLs could be documented because it is Devise's feature. Devise makes route /users/auth/:action/callback there action is name of provider.

Fixes SUSE#697 Fixes SUSE#1108

vitoravelino · 2017-09-06T18:28:32Z

@vitoravelino I have no image there callback URLs could be documented because it is Devise's feature. Devise makes route /users/auth/:action/callback there action is name of provider.

I know. What I mean is that when we are creating the oauth application on the providers' websites (e.g.: gitlab, github, etc), we are asked about the callback redirect url. It would be nice to have somewhere (it could even be in the config.yml before each provider section), that the callback url for that provider would be <host>/users/auth/:provider/callback.

This is not required to merge the PR, this is just a nice to have thing because users might be confused when trying to configure all of this. I was confused in the beginning for example, I had to check to find something related to callback.

Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

mssola

Portus crashed on bitbucket support if the team option is left empty.

mssola · 2017-09-07T06:47:03Z

config/config.yml

+    domain: ""
+    options:
+      # Only members of team can sign in/up with Bitbucket. Need permission to read team membership.
+      team: ""
 # Set first_user_admin to true if you want that the first user that signs up


Leave an empty line of separation 😉

Implemented anonymous browsing

andrew2net · 2017-09-07T11:03:33Z

@mssola I got error in Travis

...
patching file Gemfile
Hunk #1 succeeded at 61 (offset 4 lines).
Hunk #2 succeeded at 80 (offset 4 lines).
Hunk #3 succeeded at 97 (offset 4 lines).
patching file Gemfile.lock
Hunk #3 FAILED at 231.
Hunk #4 succeeded at 286 (offset 26 lines).
Hunk #5 FAILED at 285.
Hunk #6 succeeded at 332 (offset 29 lines).
Hunk #7 succeeded at 405 (offset 30 lines).
Hunk #8 succeeded at 511 (offset 34 lines).
2 out of 8 hunks FAILED -- saving rejects to file Gemfile.lock.rej
The command "packaging/suse/make_spec.sh portus" exited with 255.
...

could you explain what does it mean?

mssola · 2017-09-07T11:55:29Z

@andrew2net feel free to ignore this error. It's due some packaging issues that I'm fixing in another PR

andrew2net · 2017-09-07T14:03:31Z

@mssola for me Portus with on Bitbucket and the empty team works without errors. May I have your log?

mssola · 2017-09-07T14:32:35Z

@andrew2net here it goes:

Started GET "/users/auth/bitbucket/callback?state=d7610f627c22440a22ea79f91a6a9317a9f417c203bfdcdd&code=D3ZJVMwSVVeac54HKF" for 127.0.0.1 at 2017-09-07 16:23:53 +0200
I, [2017-09-07T16:23:53.602942 #6225]  INFO -- omniauth: (bitbucket) Callback phase initiated.

Faraday::ConnectionFailed (execution expired):
  lib/omni_auth/strategies/bitbucket.rb:45:in `build_access_token_with_team_check'
  app/middleware/catch_json_parse_errors.rb:9:in `call'


  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/actionpack-4.2.8/lib/action_dispatch/middleware/templates/rescues/_source.erb (4.9ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/actionpack-4.2.8/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb (2.9ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/actionpack-4.2.8/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb (1.0ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/actionpack-4.2.8/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb within rescues/layout (24.0ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/_markup.html.erb (0.4ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/style.css.erb within layouts/inlined_string (0.4ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/_inner_console_markup.html.erb within layouts/inlined_string (0.3ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/_prompt_box_markup.html.erb within layouts/inlined_string (0.2ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/console.js.erb within layouts/javascript (14.3ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/main.js.erb within layouts/javascript (0.3ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/error_page.js.erb within layouts/javascript (0.4ms)
  Rendered /home/mssola/.rvm/gems/ruby-2.3.3/gems/web-console-2.1.3/lib/web_console/templates/index.html.erb (32.3ms)

Could it be that it's because I'm using a local development instance instead of a production reachable one ?

andrew2net · 2017-09-07T15:33:17Z

@mssola I also using local development instance. The error comes from Fraday. Looks like it is some network issue. The piece of code related to team doesn't even start. Could you try late test both with team and without?

Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

mssola

The bitbucket failure for me is a simple networking issue. Connecting to bitbucket takes ages for me. Since @vitoravelino assures me that it works for him, I'll give my 👍

I'll fix the failures reported in Travis myself.

vitoravelino

LGTM 👍

mssola · 2017-09-08T10:51:55Z

Thanks a lot for your patience and hard work ! 🎉

bufferoverflow · 2017-09-08T12:57:05Z

🎉 awesome!

Adding OAuth Support with omniauth Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

Vad1mo force-pushed the omniauth branch from 7182bed to 50d1ca4 Compare July 20, 2017 06:29

Vad1mo changed the title ~~WIP: OAuth and OpenID Connect support with omniauth~~ OAuth and OpenID Connect support with omniauth Jul 30, 2017

Vad1mo changed the title ~~OAuth and OpenID Connect support with omniauth~~ Add support for OAuth/OpenID Connect Jul 30, 2017

mssola suggested changes Aug 1, 2017

View reviewed changes

mssola requested a review from vitoravelino August 1, 2017 12:01

mssola added this to the Release 2.3 milestone Aug 2, 2017

vitoravelino reviewed Aug 10, 2017

View reviewed changes

vitoravelino suggested changes Aug 10, 2017

View reviewed changes

Vad1mo force-pushed the omniauth branch 3 times, most recently from dda7f98 to cd9e640 Compare August 17, 2017 23:32

Adding OAuth Support with omniauth

4311b11

Vad1mo force-pushed the omniauth branch from cd9e640 to 4311b11 Compare August 23, 2017 22:01

Vad1mo force-pushed the omniauth branch from 4311b11 to f32f9e0 Compare August 29, 2017 20:38

vitoravelino reviewed Sep 6, 2017

View reviewed changes

config/config.yml Outdated

# Gitlab authentication support.

gitlab:

enabled: false

id: ""

Copy link

Contributor

vitoravelino Sep 6, 2017

Choose a reason for hiding this comment

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

Here could be application_id

vitoravelino suggested changes Sep 6, 2017

View reviewed changes

vitoravelino added 2 commits September 6, 2017 15:18

explore: improved user interface

9d6cc25

js: created <tag> component with copy to clipboard

acad5b6

Fixes SUSE#697 Fixes SUSE#1108

andrew2net and others added 3 commits September 6, 2017 21:05

fix PR issues

9d5d30d

Adding OAuth Support with omniauth

475a499

Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

Merge branch 'omniauth' into wip_oauth

e1b0dcb

Vad1mo force-pushed the omniauth branch from 4e99e87 to 475a499 Compare September 6, 2017 21:03

mssola suggested changes Sep 7, 2017

View reviewed changes

andrew2net and others added 2 commits September 7, 2017 09:05

add line separation in config

6b74021

Merge pull request SUSE#1377 from mssola/anonymous-browsing

70188d5

Implemented anonymous browsing

Vad1mo added 2 commits September 7, 2017 18:20

Adding OAuth Support with omniauth

0c8c07b

Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

Adding OAuth Support with omniauth

a25c866

Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

mssola approved these changes Sep 8, 2017

View reviewed changes

vitoravelino approved these changes Sep 8, 2017

View reviewed changes

mssola merged commit 0a5fefd into SUSE:master Sep 8, 2017

mssola pushed a commit to mssola/Portus that referenced this pull request Sep 12, 2017

Add support for OAuth/OpenID Connect (SUSE#1300)

39abe60

Adding OAuth Support with omniauth Signed-off-by: Vadim Bauer <bauer.vadim@gmail.com>

Vad1mo deleted the omniauth branch February 6, 2018 21:53

Add support for OAuth/OpenID Connect #1300

Add support for OAuth/OpenID Connect #1300

Conversation

Vad1mo commented Jun 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssola commented Aug 1, 2017

mssola commented Aug 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitoravelino Aug 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitoravelino commented Aug 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitoravelino commented Aug 10, 2017

vitoravelino left a comment

Choose a reason for hiding this comment

andrew2net commented Aug 11, 2017 • edited Loading

Vad1mo commented Aug 12, 2017

Vad1mo commented Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitoravelino commented Sep 6, 2017

vitoravelino commented Sep 6, 2017 • edited Loading

vitoravelino left a comment

Choose a reason for hiding this comment

andrew2net commented Sep 6, 2017

vitoravelino commented Sep 6, 2017

mssola left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew2net commented Sep 7, 2017

mssola commented Sep 7, 2017

andrew2net commented Sep 7, 2017 • edited Loading

mssola commented Sep 7, 2017

andrew2net commented Sep 7, 2017

mssola left a comment

Choose a reason for hiding this comment

vitoravelino left a comment

Choose a reason for hiding this comment

mssola commented Sep 8, 2017 • edited Loading

bufferoverflow commented Sep 8, 2017

Vad1mo commented Jun 10, 2017 •

edited

Loading

vitoravelino Aug 10, 2017 •

edited

Loading

vitoravelino commented Aug 10, 2017 •

edited

Loading

andrew2net commented Aug 11, 2017 •

edited

Loading

Vad1mo commented Aug 24, 2017 •

edited

Loading

vitoravelino commented Sep 6, 2017 •

edited

Loading

andrew2net commented Sep 7, 2017 •

edited

Loading

mssola commented Sep 8, 2017 •

edited

Loading