-
Notifications
You must be signed in to change notification settings - Fork 52
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
Adapting for CodeIgniter 3 #47
Comments
@donjakobo I see that you have started a branch for CI3. Do you want me to merge what I have for CI3 into it (there are some changes you should check out before I do that, namely the separation of admin panel and user account files)? |
@AdwinTrave yes please merge your CI3, I don't want to put it into the main one because CI3 is not yet official. So until then lest keep it as an optional branch. Sorry I've been wrapped up lately, got a wedding to plan out |
@donjakobo Will do. Congrats! Update: Merge requests submitted. |
While writing a small guide for A3M I have taken a closer look on the sign in functionality and found that the way it's probably not the best and makes it harder to use it without the demo page and if improperly used it can pose quiet a security risk. I have made a few changes to it so that now all the authentication stuff is part of one method. Well I guess it would be better to show the code: What does everyone think? @donjakobo |
Finally fixed the session extension for CI3 and pretty much everything else except #30 has been done/fixed. I also added version check for the sessions so everything should be backward compatible to CI2. I will submit a merge request once I get some feedback on the proposed authentication improvement above. @donjakobo After that I would say that only few little improvements need to be done and we can happily port the improvements into the CI2 branch. Then if we are going to wait for the official CI3 release to consider it stable I will port in Bootstrap 3 and fix the theme for it. Well, what I am getting at is, that we could use some roadmap or plan where this is going (although I realize that is kinda silly when we don't know where CodeIgniter is going). |
Update: We've done a bit of AJAX for sign up at my work, so I'll be convincing my co-worker who did it to adapt push it up to A3M as well. Also I plan to put in Bootstrap 3 now that it came out. |
Nice :) I still think that it would be best to stay on CI2.X until 3.X is official and stable. What are your thougths @AdwinTrave ? Are you using this in a production setting already? |
@donjakobo I agree. Stay with 2.X till 3.X is ready, but if somebody wants to use 3.X it's stable now. The only thing you need to do is follow up with the development and the hassle of adjusting things if something changes. Since I'm doing the same thing with A3M and my other two projects (currently still in development) where I'm using A3M it will allow us to be ready and have some new features one CI 3.X is released. |
On the note of design in the 2.X branch I would stick with Bootstrap 2.X and for the 3.X I would upgrade to Bootstrap 3.X and would like to work on a new theme. Update: I have added confirm password for sign up: http://a3m.freedombase.net/register |
Progress report: moving things around authentication so that things are a bit easier to use for different implementations. If CI's CSRF protection is one A3M will not work for some strange reason (need to figure out and fix). I think there is similar issue with secure cookies and so on (again this needs to be fixed). |
@donjakobo After #83 The only things remaining on my list it to create a native session driver for A3M and improving existing features. |
Just tested CSRF protection. It no longer causes troubles. So only the native session driver remains. |
@donjakobo We should rethink the use of |
@AdwinTrave yeah I don't even use |
It is gone. Now then. CI3 is progressing smoothly, but the last feature that is to be implemented will cause us some issues: bcit-ci/CodeIgniter#3073 |
@donjakobo |
@AdwinTrave About adapting MY_Session::keep_flashdata() to the new core Session library, you may have a look at this: |
Thanks! That will be helpful. If you know how to do it for A3M and could submit a PR to this or my repo that would be great. |
I am sorry, there is no time for me to participate here directly. At least it would not happen soon. I am adapting code of A3M within a project, but I am rewriting too much to fit to my platform. I use base controllers and I am moving the redirection logic there, for example. Another thing that is going to be a problem is the "cookie_monster" - the autologin feature. I simply don't understand how it works. I am going to replace it using code from this project: https://github.com/jenssegers/codeigniter-authentication-library Here are some articles: http://jenssegers.be/blog/12/codeigniter-authentication-library-1-3 , http://fishbowl.pastiche.org/2004/01/19/persistent_login_cookie_best_practice/ , http://jaspan.com/improved_persistent_login_cookie_best_practice Sorry, again. I am too busy. |
Understandable. The issue is to figure out how we can do with as less extending of the original libraries. cookie_monster will have to be deprecated due to the fact that cookies will no longer be used. |
A suggestion: Within the configuration file account.php some parameters for username validation may be included. For example: $config['username_min_length'] = 2;
$config['username_max_length'] = 24;
$config['username_validator'] = '/^[A-Za-z][A-Za-z0-9\-_]*$/';
$config['password_min_length'] = 6; Now these things are hard-coded. |
I like the idea except for username_validator as sometimes you want to have some special characters allowed. I think the current build-in checks in the CI should suffice. |
On the previous topic. I managed to get the new sessions working without any need for extending the library. If possible I would like to keep it that way. This also means that A3M would be using database session as default. |
Status report:
What is still left:
Would be nice:
After those two things are done, I will release a beta for A3M v2 and we'll be ready for official CI3 release. |
CodeIgniter 3RC3 update close #47
New CI sessions seem to be working well. |
As the development of CodeIgniter 3 is nearing its end I think it's important to account for the changes in CI3. In my fork I started a new branch with CI3. https://github.com/AdwinTrave/A3M/tree/CI3
The first issue is the change of the session library to drivers which doesn't allow us to use the session library extension any more. This currently mainly means that Remember me functionality is not working.
I also encountered an issue when loading multiple models in an array gives an error when trying to call functions from those models (e.g. Fatal error: Call to a member function get_by_id() on a non-object in), I originally though this was a CI3 issue, but I failed every time I tried to replicate it with my own models, so I have no clue what the issue is. Loading the models individually seems to fix the issue. Problem is if you autoload these models.
Next when attempting to connect to OpenID site I get page full of runtime errors (e.g. Non-static method Auth_OpenID_Message::fromOpenIDArgs() should not be called statically, assuming $this from incompatible context) since it's similar to the models error I think those might be related somehow.
Lastly the Facebook redirect, sometimes the Facebook server won't respond so for my project I just got the all.js from Facebook and placed it on my server, which works great, although I would like to figure out how to improve this even further.
In general, while working on this I plan to add #30 , make sure that all pages are HTML5 valid and many more minor improvements. Hopefully I'll be able to fix everything so that it is all backward compatible with CI 2.
I'm using A3M on a project for this summer so I plan to have most of this done by the end of summer.
My testing site: http://a3m.freedombase.net/
Any help would be greatly appreciated.
The text was updated successfully, but these errors were encountered: