[api] new user register and change_password commands

docs/api/api/api.txt

@@ -1,6 +1,6 @@
 = openSUSE API
 
-Version: 2.3
+Version: 2.4
 
   Only authenticated users are allowed to access the API. Authentication is done
   by sending a Basic HTTP Authorisation header. 
@@ -132,45 +132,32 @@ XmlBody: user
 XmlResult: status
 
 
-GET /person/<userid>/<groupid>
+POST /person?cmd=register
 
-  Read user and group data.
+  Can be used to register a new user, if OBS instance is allowing this.
 
-XmlResult: TODO
 
+POST /person/<userid>
 
-POST /person/register
+  cmd=change_password to post the new password in the request body. Just the first line gets used.
 
-  TODO
+XmlBody: password
+XmlResult: status
 
 Parameters:
-  TODO
-
-
-PUT /person/register
-
-  TODO
-
-XmlBody: TODO
-XmlResult: TODO
-
+  cmd: change_password
 
-PUT /person/changepasswd
 
-  TODO
-
-XmlBody: TODO
-XmlResult: TODO
+== Group data
 
+GET /group
 
-POST /person/changepasswd
+  List available groups
 
-  TODO
+XmlResult: directory
 
 Parameters:
-  TODO
-
-== Group data
+  login: List available groups of this user.
 
 GET /group/<group title>
 

src/api/app/controllers/application_controller.rb

@@ -33,7 +33,7 @@ class ApplicationController < ActionController::API
   end
 
   # skip the filter for the user stuff
-  before_filter :extract_user, :except => :register
+  before_filter :extract_user
   before_filter :setup_backend
   before_filter :shutup_rails
   before_filter :set_current_user

src/api/app/controllers/person_controller.rb

@@ -8,33 +8,50 @@ class PersonController < ApplicationController
   validate_action :register => {:method => :put, :response => :status}
   validate_action :register => {:method => :post, :response => :status}
 
+  skip_before_filter :extract_user, :only => [:index, :register]
+
   # Returns a list of all users (that optionally start with a prefix)
   def index
-    valid_http_methods :get
+    valid_http_methods :get, :post
 
-    if !@http_user
-      logger.debug "No user logged in, permission to index denied"
-      @errorcode = 401
-      @summary = "No user logged in, permission to index denied"
-      render :template => 'error', :status => @errorcode
-      return
-    end
+    unless request.post? and params[:cmd] == "register"
+      extract_user
 
-    if params[:prefix]
-      list = User.where("login LIKE ?", params[:prefix] + '%').all
-    else
-      list = User.all
+      if !@http_user 
+        logger.debug "No user logged in, permission to index denied"
+        render_error :status => 401, :errorcode => "unknown_user",
+                     :message => "No anonymous access allowed"
+        return
+      end
     end
 
-    builder = Builder::XmlMarkup.new(:indent => 2)
-    xml = builder.directory(:count => list.length) do |dir|
-      list.each {|user| dir.entry(:name => user.login)}
+    if request.get?
+      if params[:prefix]
+        list = User.where("login LIKE ?", params[:prefix] + '%').all
+      else
+        list = User.all
+      end
+
+      builder = Builder::XmlMarkup.new(:indent => 2)
+      xml = builder.directory(:count => list.length) do |dir|
+        list.each {|user| dir.entry(:name => user.login)}
+      end
+      render :text => xml, :content_type => "text/xml"
+      return
+    elsif request.post?
+      if params[:cmd] == "register"
+        internal_register
+        return
+      else
+        render_error :status => 400, :errorcode => "unknown_command",
+                     :message => "Allow commands are 'change_password'"
+        return
+      end
     end
-    render :text => xml, :content_type => "text/xml"
   end
 
   def userinfo
-    valid_http_methods :get, :put
+    valid_http_methods :get, :put, :post
 
     if !@http_user
       logger.debug "No user logged in, permission to userinfo denied"
@@ -60,6 +77,28 @@ def userinfo
         logger.debug "Generating user info for logged in user #{@http_user.login}"
         render :text => @http_user.render_axml(true), :content_type => "text/xml"
       end
+    elsif request.post?
+      if params[:cmd] == "change_password"
+        login ||= @http_user.login
+        password = request.raw_post.to_s.chomp
+        if login != @http_user.login and not @http_user.is_admin?
+          render_error :status => 403, :errorcode => "change_password_no_permission",
+                       :message => "No permission to change password for user #{login}"
+          return
+        end
+        if password.blank?
+          render_error :status => 404, :errorcode => "password_empty",
+                       :message => "No new password given in first line of the body"
+          return
+        end
+        change_password(login, password)
+        render_ok
+        return
+      else
+        render_error :status => 400, :errorcode => "unknown_command",
+                     :message => "Allow commands are 'change_password'"
+        return
+      end
     elsif request.put?
       if user and user.login != @http_user.login and !@http_user.is_admin?
         logger.debug "User has no permission to change userinfo"
@@ -108,8 +147,12 @@ def grouplist
   end
 
   def register
+    # FIXME 3.0, to be removed
     valid_http_methods :post, :put
+    internal_register
+  end
 
+  def internal_register
     if CONFIG['ldap_mode'] == :on
       render_error :message => "LDAP mode enabled, users can only be registered via LDAP", :errorcode => "err_register_save", :status => 400
       return
@@ -232,14 +275,19 @@ def change_my_password
     password = xml.elements["/userchangepasswd/password"].text
     login = URI.unescape(login)
 
+    change_password(login, URI.unescape(password))
+    render_ok
+  end
+
+  def change_password(login, password)
     if !@http_user
       logger.debug "No user logged in, permission to changing password denied"
       @errorcode = 401
       @summary = "No user logged in, permission to changing password denied"
       render :template => 'error', :status => 401
     end
 
-    if not login or not password
+    if login.blank? or password.blank?
       render_error :status => 404, :errorcode => 'failed to change password',
             :message => "Failed to change password: missing parameter"
       return
@@ -249,16 +297,15 @@ def change_my_password
             :message => "No sufficiend permissions to change password for others"
       return
     end
-
-    newpassword = Base64.decode64(URI.unescape(password))
 
     #change password to LDAP if LDAP is enabled    
     if CONFIG['ldap_mode'] == :on
+      ldap_password = Base64.decode64(password)
       if CONFIG['ldap_ssl'] == :on
         require 'base64'
         begin
           logger.debug( "Using LDAP to change password for #{login}" )
-          result = User.change_password_ldap(login, newpassword)
+          result = User.change_password_ldap(login, ldap_password)
         rescue Exception
           logger.debug "CONFIG['ldap_mode'] selected but 'ruby-ldap' module not installed."
         end
@@ -274,11 +321,9 @@ def change_my_password
 
     #update password in users db
     @user = User.get_by_login(login)
-    logger.debug("find the user")
-    @user.password = newpassword
-    @user.password_confirmation = newpassword
-    @user.state = User.states['confirmed']
+    @user.update_password( password )
     @user.save!
-    render_ok
   end
+  private :change_password
+
 end

src/api/config/routes.rb

@@ -11,14 +11,12 @@
 
     ### /person
     match 'person' => 'person#index'
-    # FIXME: this is no clean namespace, a person "register" or "changepasswd" could exist ...
-    #        suggested solution is POST person/:login?cmd=register
-    #        fix this for OBS 3.0
-    match 'person/register' => 'person#register'
-    match 'person/changepasswd' => 'person#change_my_password'
-    # bad api, to be removed for OBS 3. Use /group?person=:login instead
-    match 'person/:login/group' => 'person#grouplist', :constraints => cons
-
+    # FIXME3.0: this is no clean namespace, a person "register" or "changepasswd" could exist ...
+    #           remove these for OBS 3.0
+    match 'person/register' => 'person#register'                            # use /person?cmd=register POST instead
+    match 'person/changepasswd' => 'person#change_my_password'              # use /person/:login?cmd=changepassword POST instead
+    match 'person/:login/group' => 'person#grouplist', :constraints => cons # Use /group?person=:login GET instead
+    # /FIXME3.0
     match 'person/:login' => 'person#userinfo', :constraints => cons
 
     ### /group

src/api/test/functional/person_controller_test.rb

@@ -5,11 +5,8 @@ class PersonControllerTest < ActionController::IntegrationTest
 
   fixtures :all
 
-  def setup
-    prepare_request_valid_user
-  end
-
   def test_index
+    prepare_request_with_user "adrian", "so_alone"
     get "/person"
     assert_response :success
 
@@ -18,22 +15,26 @@ def test_index
   end
 
   def test_ichain
+    prepare_request_with_user "adrian", "so_alone"
     get "/person/tom", nil, { "username" => "fred" }
     assert_response :success
   end
 
   def test_userinfo_for_valid_http_user
+    prepare_request_with_user "adrian", "so_alone"
     get "/person/tom"
     assert_response :success   
     # This returns the xml content with the user info
   end
 
   def test_userinfo_from_param_valid
+    prepare_request_with_user "adrian", "so_alone"
     get "/person/fred"
     assert_response :success
   end
 
   def test_userinfo_from_param_invalid
+    prepare_request_with_user "adrian", "so_alone"
     get "/person/notfred"
     assert_response 404 
   end
@@ -114,7 +115,57 @@ def test_update_user_info
     assert_no_xml_tag :tag => 'person', :child => {:tag => 'globalrole', :content => "Admin"}
   end
 
-  def test_register
+  def test_register_and_change_password_new_way
+    reset_auth
+    data = '<unregisteredperson>
+              <login>adrianSuSE</login>
+              <email>adrian@suse.de</email>
+              <realname>Adrian Schroeter</realname>
+              <state>locked</state>
+              <password>so_alone</password>
+              <note>I do not trust this guy, this note is only allowed to be stored by admin</note>
+            </unregisteredperson>"
+           '
+    post "/person?cmd=register", data
+    assert_response :success
+
+    u = User.find_by_login "adrianSuSE"
+    assert_not_nil u
+    assert_equal u.login, "adrianSuSE"
+    assert_equal u.email, "adrian@suse.de"
+    assert_equal u.realname, "Adrian Schroeter"
+    assert_equal u.adminnote, ""
+
+    # change password
+    data = 'NEWPASSW0RD'
+    post "/person/adrianSuSE?cmd=change_password", data
+    assert_response 401
+
+    # wrong user
+    prepare_request_with_user "adrian", "so_alone"
+    post "/person/adrianSuSE?cmd=change_password", data
+    assert_response 403
+    assert_xml_tag :tag => 'status', :attributes => { :code => "change_password_no_permission" }
+
+    # admin
+    prepare_request_with_user "king", "sunflower"
+    post "/person/adrianSuSE?cmd=change_password", ""
+    assert_response 404
+    assert_xml_tag :tag => 'status', :attributes => { :code => "password_empty" }
+
+    post "/person/adrianSuSE?cmd=change_password", data
+    assert_response :success
+
+    u = User.find_by_login "adrianSuSE"
+    assert_not_nil u
+    assert_equal u.login, "adrianSuSE"
+    assert_equal u.password, data
+
+    #cleanup
+    u.destroy
+  end
+
+  def test_register_old_way
     reset_auth
     data = '<unregisteredperson>
               <login>adrianSuSE</login>
@@ -125,6 +176,7 @@ def test_register
               <note>I do not trust this guy, this note is only allowed to be stored by admin</note>
             </unregisteredperson>"
            '
+    # FIXME3.0: to be removed
     post "/person/register", data
     assert_response :success